[Asterisk-code-review] res_pjsip_session.c: Check topology on re-invite. (asterisk[16])

[George Joseph]

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

Change subject: res_pjsip_session.c: Check topology on re-invite.
......................................................................


Patch Set 1:

(3 comments)

https://gerrit.asterisk.org/c/asterisk/+/15445/1/res/res_pjsip_session.c 
File res/res_pjsip_session.c:

https://gerrit.asterisk.org/c/asterisk/+/15445/1/res/res_pjsip_session.c@2405 
PS1, Line 2405: 			if (session->active_media_state->topology) {
This check requires that session->active_media_state->topology be present which, of course, means that session->active_media_state must be non-NULL.   Now see below...


https://gerrit.asterisk.org/c/asterisk/+/15445/1/res/res_pjsip_session.c@2445 
PS1, Line 2445: if this re-invite was the result of a requested topology
              : 				 * change.  If it was the result of some other change, like connected line, then
              : 				 * we don't want to suppress it even though the topologies are equal.
We should remove this comment if it no longer applies.


https://gerrit.asterisk.org/c/asterisk/+/15445/1/res/res_pjsip_session.c@2449 
PS1, Line 2449: 				if ((topology_change_request || session->active_media_state) && ast_stream_topology_equal(session->active_media_state->topology, pending_media_state->topology)) {
> As far as connected line updates go, I ran an attended_transfer testsuite test with and without this […]
It's possible that I'm looking at it wrong but...  We're still inside the block that requires that session->active_media_state be non-NULL.  In turn, doesn't that mean that "(topology_change_request || session->active_media_state)" will always be true?  If so, you can just remove that extra predicate and just use ast_stream_topology_equal to determine if we should bail at this point.



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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I04dc91befb2387904e28a9aaeaa3bcdbcaa7fa63
Gerrit-Change-Number: 15445
Gerrit-PatchSet: 1
Gerrit-Owner: Benjamin Keith Ford <bford at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-CC: Kevin Harwell <kharwell at digium.com>
Gerrit-CC: Stanislav Abramenkov <stas.abramenkov at gmail.com>
Gerrit-Comment-Date: Fri, 19 Feb 2021 13:45:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: George Joseph <gjoseph 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/20210219/b561918a/attachment.html>