[Asterisk-code-review] core: Remove embedded module support (asterisk[master])
Corey Farrell
asteriskteam at digium.com
Fri Mar 24 14:52:19 CDT 2017
Corey Farrell has posted comments on this change. ( https://gerrit.asterisk.org/5314 )
Change subject: core: Remove embedded module support
......................................................................
Patch Set 3: Code-Review-1
(4 comments)
Can't speak for others but I like this. Mostly just nit-picks (except AST_MODULE_INFO for C++).
Other notes:
* I believe embedding does actually work for some modules if you have an old enough gcc (like CentOS 5 maybe). I don't think this matters in master, do we care in 13/14?
* Does this need mention in CHANGES? I'm thinking no but worth 2 seconds of thought.
* At some point we should close/wont-fix ASTERISK-20761.
https://gerrit.asterisk.org/#/c/5314/3/Makefile.moddir_rules
File Makefile.moddir_rules:
Line 22
This flag and GC_LDFLAGS can also be removed from configure.ac / makeopts.in.
Line 144
Maybe this should be left in place like '*.eo' a few lines above?
https://gerrit.asterisk.org/#/c/5314/3/include/asterisk/module.h
File include/asterisk/module.h:
PS3, Line 380: NULL, \
: NULL, \
These two NULL's corresponded to backup_globals / restore_globals so they need to be removed.
PS3, Line 441: #define __MODULE_INFO_SECTION
: #define __MODULE_INFO_GLOBALS
Nit: These defines were for use by the AST_MODULE_INFO macro only, why not just remove them here and there?
--
To view, visit https://gerrit.asterisk.org/5314
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I13206123c371f4d84d8c9d3c36fea816dc57aeb2
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-HasComments: Yes
More information about the asterisk-code-review
mailing list