[Asterisk-code-review] Change in asterisk[master]: res_pjsip: Refactor endpt_send_request to include transactio...

[Joshua Colp (Code Review)]

Joshua Colp has posted comments on this change.

Change subject: res_pjsip: Refactor endpt_send_request to include transaction timeout
......................................................................


Patch Set 4: Code-Review-1

(3 comments)

This change increases our minimum required PJSIP version. Until now we've been compatible with releases since shared library support went in. Group lock support came later, as it is we can either add a configure check and require a newer PJSIP version or the code can be changed to support older versions (potentially). In the case of 13 option #1 would mean the minimum required PJSIP version would change in a release.

https://gerrit.asterisk.org/#/c/46/4/res/res_pjsip.c
File res/res_pjsip.c:

Line 2825:     void *token;
         :     void (*cb)(void*, pjsip_event*);
         : 	pjsip_transaction *tsx;
         : 	pj_timer_entry *timeout_timer;
Make these line up sanely.


Line 2845: 	if (send_tsx_module.id < 0 || event->type != PJSIP_EVENT_TSX_STATE) {
It shouldn't be possible for the module to not be registered and to still get called.


Line 2950: 	if (p_tsx) {
         : 		*p_tsx = tsx;
         : 	}
I think the only way this transaction is guaranteed to remain valid is by virtue of transactions not being destroyed until a period in the future. I'm not sure I'm comfortable with that being the only guarantee. Keeping the group lock, on the other hand, would guarantee it.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0778dc730d9689c5147a444a04aee3c1026bf747
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph <george.joseph at fairview5.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-HasComments: Yes