[Asterisk-code-review] chan pjsip: Creating Channel Causes Asterisk to Crash When D... (asterisk[master])

[Matt Jordan]

Matt Jordan has posted comments on this change.

Change subject: chan_pjsip: Creating Channel Causes Asterisk to Crash When Duplicate AOR Sections Exist in pjsip.conf
......................................................................


Patch Set 1: Code-Review-1

(4 comments)

https://gerrit.asterisk.org/#/c/254/1//COMMIT_MSG
Commit Message:

Line 7: chan_pjsip: Creating Channel Causes Asterisk to Crash When Duplicate AOR Sections Exist in pjsip.conf
I'd try to knock this down to 80 characters or less. How about:

chan_pjsip: Fix crash during channel creation in the presence of duplicate AORs

(80 exact)


https://gerrit.asterisk.org/#/c/254/1/res/res_sorcery_config.c
File res/res_sorcery_config.c:

Line 297: 		/* Confirm an object with this id does not already exist in the bucket. If so, the configuration is invalid so, destroy it and stop processing. */
Granted, this may not be consistent everywhere, but I'd try to wrap the code comments at 90 columns (which is the line length set in the coding guidelines).


Line 298: 		if ((obj = ao2_find(objects, id, OBJ_KEY))) {
Pre-emptive Richard Mudgett comment:

Prefer doing assignments out of conditional evaluations:

obj = ao2_find(objects, id, OBJ_KEY);
if (obj) {
 ...
}


Line 299: 			ast_log(LOG_ERROR, "Config file '%s' could not be loaded; configuration contains a duplicate object: '%s' of type '%s'\n",
        : 				config->filename, id, type);
        : 			ast_config_destroy(cfg);
        : 			return;
ao2_find increases the reference count on obj. Make sure you drop the reference count in this path, otherwise it will be leaked:

obj = ao2_find(...)
if (obj) {
    ast_log(LOG_ERROR, ...);
    ao2_ref(obj, -1);
    ast_config_destroy(cfg);
    return;
}


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I35090ca4cd40f1f34881dfe701a329145c347aef
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes