[Asterisk-code-review] Change in asterisk[13]: pjsip_options: Add qualify_timeout processing and eventing

[Joshua Colp (Code Review)]

Joshua Colp has posted comments on this change.

Change subject: pjsip_options: Add qualify_timeout processing and eventing
......................................................................


Patch Set 3: Code-Review-1

(8 comments)

https://gerrit.asterisk.org/#/c/44/3//COMMIT_MSG
Commit Message:

Line 23: OPTIONS mesasge.  The default is 3000ms.  When the timer expires, the contact is
OPTIONS message.


https://gerrit.asterisk.org/#/c/44/3/CHANGES
File CHANGES:

Line 32: This was previously
       :    hard coded to 32 seconds in pjproject.
Hrm. This previously used a hard coded value of 32 seconds in pjproject.

I don't want it to sound as though we made it dynamic within pjproject itself. #pedantic


https://gerrit.asterisk.org/#/c/44/3/include/asterisk/res_pjsip.h
File include/asterisk/res_pjsip.h:

Line 916:  * \retval non-NULL if contacts available
I'd also make it explicit that the container has to be freed - it should be obvious and other APIs in this area are lax on such things, but better to be explicit!


Line 918: struct ao2_container *ast_sip_location_retrieve_contacts_from_aor_list(char *aor_list);
const char *


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

Line 1014: 						If the contact doesn't repond before the timeout,
I'd expand this with more detail.

If the contact doesn't respond to the OPTIONS request within the timeout,


https://gerrit.asterisk.org/#/c/44/3/res/res_pjsip/location.c
File res/res_pjsip/location.c:

Line 194: struct ao2_container *ast_sip_location_retrieve_contacts_from_aor_list(char *aor_list)
const char *


https://gerrit.asterisk.org/#/c/44/3/res/res_pjsip/pjsip_configuration.c
File res/res_pjsip/pjsip_configuration.c:

Line 96: 		ao2_cleanup(contacts);
ao2_ref, contacts can't be NULL


https://gerrit.asterisk.org/#/c/44/3/res/res_pjsip/pjsip_options.c
File res/res_pjsip/pjsip_options.c:

Line 119: 	update = ast_sorcery_copy(ast_sip_get_sorcery(), status);
If you can... copying yourself is faster. The ast_sorcery_copy operation goes object -> object set -> object


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id0ce0528e58014da1324856ea537e7765466044a
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes