[Asterisk-code-review] loader: Convert reload classes to built-in modules. (asterisk[master])

[Richard Mudgett]

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

Change subject: loader: Convert reload_classes to built-in modules.
......................................................................


Patch Set 1:

(4 comments)

https://gerrit.asterisk.org/#/c/8390/1/include/asterisk/module.h
File include/asterisk/module.h:

https://gerrit.asterisk.org/#/c/8390/1/include/asterisk/module.h@295
PS1, Line 295: 	AST_MODPRI_CORE =               40,  /*!< A core module originally meant to start between preload and load. */
> I don't think so, I'm not aware of any dependencies between timing modules 
I think for safety we should make room by adding the 10 as you have indicated.


https://gerrit.asterisk.org/#/c/8390/1/main/cel.c
File main/cel.c:

https://gerrit.asterisk.org/#/c/8390/1/main/cel.c@1568
PS1, Line 1568: 		return AST_MODULE_LOAD_FAILURE;
> I think this is inconsistent.  We do not run unload_module on 'core stop no
Many modules actually do cleanup as best they can when they are aborting the load.  res_pjsip.c and chan_sip.c do.


https://gerrit.asterisk.org/#/c/8390/1/main/loader.c
File main/loader.c:

https://gerrit.asterisk.org/#/c/8390/1/main/loader.c@509
PS1, Line 509: 		mod = ast_calloc(1, sizeof(*mod) + strlen(info->name) + 1);
> Hmm I think I'll have to ast_std_calloc here, then when built-in modules ar
You don't have to recreate it.  You can use the builtin flag to release it using ast_std_free if set and ast_free otherwise.


https://gerrit.asterisk.org/#/c/8390/1/res/res_config_sqlite.c
File res/res_config_sqlite.c:

https://gerrit.asterisk.org/#/c/8390/1/res/res_config_sqlite.c@1780
PS1, Line 1780: 	.requires = "cdr",
> This module is funny as it registers a realtime driver, loads config and re
Then the dependencies should be:

.requires = "extconfig",
.optional_modules = "cdr",

The module was just deprecated in the middle of February so the support level being wrong is understandable.  This points out that the recently deprecated modules may not have updated the AST_MODULE_INFO status to match.



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

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I371a9a45064f20026c492623ea8062d02a1ab97f
Gerrit-Change-Number: 8390
Gerrit-PatchSet: 1
Gerrit-Owner: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Thu, 08 Mar 2018 19:43:38 +0000
Gerrit-HasComments: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180308/71f0d732/attachment.html>