[Asterisk-code-review] loader: Add dependency fields to module structures. (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/7872 )

Change subject: loader: Add dependency fields to module structures.
......................................................................


Patch Set 5:

(5 comments)

https://gerrit.asterisk.org/#/c/7872/5/include/asterisk/module.h
File include/asterisk/module.h:

https://gerrit.asterisk.org/#/c/7872/5/include/asterisk/module.h@342
PS5, Line 342: 	/*! Modules which must be started first, in comma-separated string format. */
             : 	const char *requires;
> "Modules which must be loaded and started first, in comma-separated string 
"Modules which must always be started first for this module to function, in comma-separated string format."


https://gerrit.asterisk.org/#/c/7872/5/res/res_agi.c
File res/res_agi.c:

https://gerrit.asterisk.org/#/c/7872/5/res/res_agi.c@4720
PS5, Line 4720: 	.requires = "res_speech",
> res_speech is not implemented with OPTIONAL_API so res_agi.so directly refe
Well, that's a surprising dependency as I've loaded that module without res_speech.so installed.  I think that is because the older version I'm using at home uses the RTLD_LAZY flag.


https://gerrit.asterisk.org/#/c/7872/5/res/res_ari.c
File res/res_ari.c:

https://gerrit.asterisk.org/#/c/7872/5/res/res_ari.c@1200
PS5, Line 1200: 	.optional_apis = "res_http_websocket",
> What about some of the basic information retrieval URL's (no use of events)
Yeah, some requests don't need websockets to respond so technically it can be optional its just not as useful.


https://gerrit.asterisk.org/#/c/7872/5/res/res_pjsip.c
File res/res_pjsip.c:

https://gerrit.asterisk.org/#/c/7872/5/res/res_pjsip.c@5128
PS5, Line 5128: 	.requires = "res_pjproject,res_pjsip_config_wizard",
> Maybe it should move to .optional_apis?  I know that the config wizard need
res_pjsip_config_wizard does need to be loaded and running before res_pjsip starts.  However, all it's work is done using sorcery observers and wizards.  It does not have to be loaded for res_pjsip to function if you are not using the config wizard for configuration.


https://gerrit.asterisk.org/#/c/7872/5/res/res_pjsip_outbound_publish.c
File res/res_pjsip_outbound_publish.c:

https://gerrit.asterisk.org/#/c/7872/5/res/res_pjsip_outbound_publish.c@1705
PS5, Line 1705: 	.requires = "res_pjproject,res_pjsip",
> This module directly calls ast_pjproject_get_buildopt.  You are right it's 
Ok.  That makes sense.



-- 
To view, visit https://gerrit.asterisk.org/7872
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ad9547a0a6442409ff4e352a6d897bef2cc04bf
Gerrit-Change-Number: 7872
Gerrit-PatchSet: 5
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Mon, 15 Jan 2018 01:55:04 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180114/c3c3d80d/attachment.html>