[Asterisk-code-review] res/res pjsip session: Check for presence of an active negot... (asterisk[13])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: res/res_pjsip_session: Check for presence of an active negotiator
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.asterisk.org/#/c/3111/1/res/res_pjsip_session.c
File res/res_pjsip_session.c:

Line 921: 		if (inv_session->neg && pjmedia_sdp_neg_get_state(inv_session->neg) != PJMEDIA_SDP_NEG_STATE_DONE) {
> I'm not sure I understand this comment.
My point is if inv_session->neg is NULL then we are trying to refresh the session so early that the initial INVITE or response to the INVITE hasn't actually been built yet much less sent.  In that case we could just return to let the other thread do its job and the refresh will be part of the initial INVITE negotiation or we could avoid any race potential and delay the refresh request.

Furthermore, pjmedia_sdp_neg_get_state() is NULL tolerant because of an immediate PJPROJECT PJ_ASSERT_RETURN() check.  If asserts are not enabled it returns the negotiation state as not done.  If asserts are enabled it aborts Asterisk.

This change either doesn't even need to be done or the checks should be done with '||' instead of '&&' to avoid a possible PJPROJECT assert aborting Asterisk.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1028323e7e01b0a531865e5412a71b6f6ec4276d
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes