[Asterisk-code-review] res pjsip: Make system and global memory cache friendly (asterisk[13])

Richard Mudgett asteriskteam at digium.com
Fri Feb 5 16:33:16 CST 2016


Richard Mudgett has posted comments on this change.

Change subject: res_pjsip: Make system and global memory_cache friendly
......................................................................


Patch Set 3: Code-Review-1

(6 comments)

https://gerrit.asterisk.org/#/c/2156/3/res/res_pjsip/config_global.c
File res/res_pjsip/config_global.c:

Line 107: 		}
        : 		cfg = ao2_find(globals, NULL, 0);
Nit.  Lost the blank line that was between these lines.


Line 264: 		}
        : 	}
globals is ref leaked when count == 0.


https://gerrit.asterisk.org/#/c/2156/3/res/res_pjsip/config_system.c
File res/res_pjsip/config_system.c:

Line 179: 		}
        : 	}
systems is ref leaked when count == 0.


Line 216: 	RAII_VAR(struct ao2_container *, system_configs, NULL, ao2_cleanup);
        : 	RAII_VAR(struct system_config *, system, NULL, ao2_cleanup);
These are not used any more.

This is another reason to try avoiding RAII_VAR().  The compiler cannot tell you if it is not used any more.


Line 252: 	if (ast_sorcery_instance_observer_add(system_sorcery, &observer_callbacks_system)) {
        : 		return -1;
Before returning you need to:
ast_sorcery_unref(system_sorcery);
system_sorcery = NULL;


Line 263: 	ast_sorcery_instance_observer_remove(system_sorcery, &observer_callbacks_system);
This is not tolerant of system_sorcery being NULL here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I32beaf688449808251b1eb966bb2cfcfc1d08362
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes



More information about the asterisk-code-review mailing list