[Asterisk-code-review] optional api: Remove unused nonoptreq fields (asterisk[16])

Corey Farrell asteriskteam at digium.com
Tue Sep 11 12:01:30 CDT 2018


Corey Farrell has posted comments on this change. ( https://gerrit.asterisk.org/10080 )

Change subject: optional_api: Remove unused nonoptreq fields
......................................................................


Patch Set 1: Code-Review-1

(1 comment)

https://gerrit.asterisk.org/#/c/10080/1/res/res_stasis_test.c
File res/res_stasis_test.c:

https://gerrit.asterisk.org/#/c/10080/1/res/res_stasis_test.c@108
PS1, Line 108: * Asterisk loads the module, inspects the field, then loads any needed
             :  * dependencies. This works because Asterisk passes \c RTLD_LAZY to the initial
             :  * dlopen(), which defers binding function references until they are called.
             :  *
             :  * But when you take the address of a function, that function needs to be
             :  * available at load time. So if some module used the address of
             :  * message_sink_cb() directly, and \c res_stasis_test.so wasn't loaded yet, then
             :  * that module would fail to load.
             :  *
             :  * The stasis_message_sink_cb() function gives us a layer of indirection so that
             :  * the initial lazy binding will still work as expected.
This whole comment is inaccurate for 16 / master.  The only remaining use of RTLD_LAZY is in `static int is_module_loaded(const char *)` which also uses RTLD_NOLOAD.  Actual module loads now always use RTLD_NOW and just use a loop to retry dlopen if symbols could not initially be resolved.

I'm leaning towards saying that this comment should be replaced with a statement saying that the extra layer of indirection exists because it was required by previous versions of Asterisk.

I don't know enough about this code to say if it's worth getting rid of the indirection, I lean towards just leaving the code as is (not worth changing the API/ABI).



-- 
To view, visit https://gerrit.asterisk.org/10080
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-MessageType: comment
Gerrit-Change-Id: I8df66a7007f807840414bb348511a8c14c05a9fc
Gerrit-Change-Number: 10080
Gerrit-PatchSet: 1
Gerrit-Owner: Walter Doekes <walter+asterisk at wjd.nu>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Comment-Date: Tue, 11 Sep 2018 17:01:30 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180911/dd2e6c89/attachment.html>


More information about the asterisk-code-review mailing list