[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