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

[Matt Jordan]

Matt Jordan 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 take it since we don't have a negotiator that one will be created soon?  
I'm not really sure about breaking up this line. Normally I'd agree, but breaking this up turns this:

		if (inv_session->neg && pjmedia_sdp_neg_get_state(inv_session->neg) != PJMEDIA_SDP_NEG_STATE_DONE) {


into this:

		if (inv_session->neg
			&& pjmedia_sdp_neg_get_state(inv_session->neg) != PJMEDIA_SDP_NEG_STATE_DONE) {


That actually looks a lot worse to me, particularly if you combine it with the debug statement on the next line:

		if (inv_session->neg
			&& pjmedia_sdp_neg_get_state(inv_session->neg) != PJMEDIA_SDP_NEG_STATE_DONE) {
			ast_debug(3, "Delay session refresh with new SDP to %s because SDP negotiation is not yet done...\n",
				ast_sorcery_object_get_id(session->endpoint));



Got a recommendation on how you'd like it broken up?


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