[Asterisk-code-review] sorcery/res pjsip: Refactor for realtime performance (asterisk[13])

Joshua Colp asteriskteam at digium.com
Tue Mar 15 08:13:10 CDT 2016


Joshua Colp has posted comments on this change.

Change subject: sorcery/res_pjsip:  Refactor for realtime performance
......................................................................


Patch Set 8: Code-Review-1

(5 comments)

https://gerrit.asterisk.org/#/c/2370/8/include/asterisk/sorcery.h
File include/asterisk/sorcery.h:

Line 1164:  *       of the given type. (this flag is currently ignored)
I'm not comfortable changing this in an LTS release. It will break third party stuff.


Line 1179:  *  struct ast_variable *var = ast_variable_new("qualify_frequency >", "0", "");
What happens if this fails to allocate?


https://gerrit.asterisk.org/#/c/2370/8/res/res_pjsip_registrar_expire.c
File res/res_pjsip_registrar_expire.c:

Line 57: 		sleep(check_interval);
A pthread lock/condition is actually more reliable for timing provided you enclose it in a while loop and check the return value.


Line 82: 	if (check_interval) {
Might as well take this time to add a comment explaining here that due to the threading model of observers that this can't be executed simultaneously in different threads.


https://gerrit.asterisk.org/#/c/2370/8/res/res_sorcery_realtime.c
File res/res_sorcery_realtime.c:

Line 308: 	strcpy(config->family, family);
Add a comment with /* SAFE */ for anyone reading


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id2691e447db90892890036e663aaf907b2dc1c67
Gerrit-PatchSet: 8
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes



More information about the asterisk-code-review mailing list