[Asterisk-code-review] pbx config.c: Some modules not be loaded if they initially d... (asterisk[13])

Richard Mudgett asteriskteam at digium.com
Fri Aug 17 18:33:15 CDT 2018


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

Change subject: pbx_config.c: Some modules not be loaded if they initially decline to load
......................................................................


Patch Set 5: Code-Review-1

(2 comments)

https://gerrit.asterisk.org/#/c/9883/5/pbx/pbx_config.c
File pbx/pbx_config.c:

https://gerrit.asterisk.org/#/c/9883/5/pbx/pbx_config.c@2049
PS5, Line 2049: 		local_table = ast_hashtab_create(17, ast_hashtab_compare_contexts, ast_hashtab_resize_java, ast_hashtab_newsize_java, ast_hashtab_hash_contexts, 0);
It is best if we decline the load if ast_hashtab_create() returns NULL.


https://gerrit.asterisk.org/#/c/9883/5/pbx/pbx_config.c@2051
PS5, Line 2051: 	if (!pbx_load_config(config)) {
              : 		ast_mutex_unlock(&reload_lock);
              : 		return AST_MODULE_LOAD_DECLINE;
Need to destroy local_table and set to NULL before returning.  Otherwise we will leak local_table if we then choose to exit or restart asterisk instead of attempting to load pbx_config again after fixing what caused us to decline loading pbx_config.

ast_hashtab_destroy(local_table, NULL);
local_table = NULL;

Looking at the code in pbx_load_config() and pbx_load_users(), it seems to be OK if ast_hashtab_create() fails and returns NULL.  Although it is better to check if ast_hashtab_create() failed to create local_table and decline the load.

ast_hashtab_destroy() is NULL tolerant of the passed in table.



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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib00665106043b1be5148ffa7a477396038915854
Gerrit-Change-Number: 9883
Gerrit-PatchSet: 5
Gerrit-Owner: Chris Savinovich <csavinovich at digium.com>
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-Comment-Date: Fri, 17 Aug 2018 23:33:15 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180817/4ebb7a7d/attachment.html>


More information about the asterisk-code-review mailing list