[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:

(3 comments)

https://gerrit.asterisk.org/#/c/3111/1//COMMIT_MSG
Commit Message:

Line 9: It is possible in certain situations for a session refresh to be invoked
> It's honestly a pretty weird scenario that occurred while I was playing aro
I was wanting you to elaborate in the commit message since that is where you are going to look to find an explanation of the change.  Hopefully someone would then be able to understand why something was done if it is causing a problem later.


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) {
> The negotiator will get created when we call pjsip_inv_reinvite. It will cr
Should we return rather than continuing on and creating new_sdp since it is about to get created by another thread?


Line 921: 		if (inv_session->neg && pjmedia_sdp_neg_get_state(inv_session->neg) != PJMEDIA_SDP_NEG_STATE_DONE) {
> I'm not really sure about breaking up this line. Normally I'd agree, but br
Yes break it as you have shown.  Breaking lines before operators like that gives a clue that the ast_debug statement is not part of the test even if you don't look for the opening curly brace.

Course if the line is still too long the next weakest break point should be at the '!='.

if (inv_session->neg
    && pjmedia_sdp_neg_get_state(...
       != PJMEDIA....) {
}


-- 
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: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes