[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