[Asterisk-code-review] res_pjsip_session: Handle multi-stream re-invites better (asterisk[certified/16.8])

[George Joseph]

George Joseph has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/14730 )

Change subject: res_pjsip_session:  Handle multi-stream re-invites better
......................................................................


Patch Set 3:

(4 comments)

https://gerrit.asterisk.org/c/asterisk/+/14730/3/res/res_pjsip_session.c 
File res/res_pjsip_session.c:

https://gerrit.asterisk.org/c/asterisk/+/14730/3/res/res_pjsip_session.c@2265 
PS3, Line 2265: Droped
> s/Droped/Dropped
Done


https://gerrit.asterisk.org/c/asterisk/+/14730/3/res/res_pjsip_session.c@5155 
PS3, Line 5155: 	if (!right) {
              : 		return 0;
              : 	}
> Should this return 1 as well?

Not having a "left" media session is OK because the resolver doesn't create them.  Not having a "right" one means that one was expected and not found which is an error.


https://gerrit.asterisk.org/c/asterisk/+/14730/3/res/res_pjsip_session.c@5265 
PS3, Line 5265: 	} \
> If a test fails do we want to continue with the rest of the tests?

The tests are independent so even if one should fail, later ones may pass.  Might as well get the full picture because seeing which passed and which failed could tell you what the issue is.


https://gerrit.asterisk.org/c/asterisk/+/14730/3/res/res_pjsip_session.c@5295 
PS3, Line 5295: 	test_media_add(delayed_active_state, "audio", AST_MEDIA_TYPE_AUDIO, AST_STREAM_STATE_SENDRECV, -1);
> I assume no need to check the 'test_media_add' result? If NULL is returned then CHECKER will fail later?

Exactly.  The only real failure would be an allocation failure and if it fails, you're in trouble anyway.



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14730
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: certified/16.8
Gerrit-Change-Id: Id3440972943c611a15f652c6c569fa0e4536bfcb
Gerrit-Change-Number: 14730
Gerrit-PatchSet: 3
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Comment-Date: Fri, 28 Aug 2020 16:44:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Kevin Harwell <kharwell at digium.com>
Comment-In-Reply-To: Benjamin Keith Ford <bford at digium.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200828/84ad5b5e/attachment-0001.html>