[Asterisk-code-review] res_pjsip_session: Make reschedule_reinvite check for NULL topologies (asterisk[16])

Joshua Colp asteriskteam at digium.com
Mon Mar 22 09:39:07 CDT 2021


Joshua Colp has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/15638 )

Change subject: res_pjsip_session: Make reschedule_reinvite check for NULL topologies
......................................................................

res_pjsip_session: Make reschedule_reinvite check for NULL topologies

When the check for equal topologies was added to reschedule_reinvite()
it was assumed that both the pending and active media states would
actually have non-NULL topologies.  We since discovered this isn't
the case.

We now only test for equal topologies if both media states have
non-NULL topologies.  The logic had to be rearranged a bit to make
sure that we cloned the media states if their topologies were
non-NULL but weren't equal.

ASTERISK-29215

Change-Id: I61313cca7fc571144338aac826091791b87b6e17
---
M res/res_pjsip_session.c
1 file changed, 25 insertions(+), 3 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve; Approved for Submit
  Kevin Harwell: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved



diff --git a/res/res_pjsip_session.c b/res/res_pjsip_session.c
index 412c953..0753d8e 100644
--- a/res/res_pjsip_session.c
+++ b/res/res_pjsip_session.c
@@ -4226,18 +4226,40 @@
 	struct ast_sip_session_media_state *pending_media_state = NULL;
 	struct ast_sip_session_media_state *active_media_state = NULL;
 	const char *session_name = ast_sip_session_get_name(session);
+	int use_pending = 0;
+	int use_active = 0;
+
 	SCOPE_ENTER(3, "%s\n", session_name);
 
-	/* If the two media state topologies are the same this means that the session refresh request
+	/*
+	 * If the two media state topologies are the same this means that the session refresh request
 	 * did not specify a desired topology, so it does not care. If that is the case we don't even
-	 * pass one in here resulting in the current topology being used.
+	 * pass one in here resulting in the current topology being used.  It's possible though that
+	 * either one of the topologies could be NULL so we have to test for that before we check for
+	 * equality.
 	 */
-	if (!ast_stream_topology_equal(session->active_media_state->topology, session->pending_media_state->topology)) {
+
+	/* We only want to clone a media state if its topology is not null */
+	use_pending = session->pending_media_state->topology != NULL;
+	use_active = session->active_media_state->topology != NULL;
+
+	/*
+	 * If both media states have topologies, we can test for equality.  If they're equal we're not going to
+	 * clone either states.
+	 */
+	if (use_pending && use_active && ast_stream_topology_equal(session->active_media_state->topology, session->pending_media_state->topology)) {
+		use_pending = 0;
+		use_active = 0;
+	}
+
+	if (use_pending) {
 		pending_media_state = ast_sip_session_media_state_clone(session->pending_media_state);
 		if (!pending_media_state) {
 			SCOPE_EXIT_LOG_RTN(LOG_ERROR, "%s: Failed to clone pending media state\n", session_name);
 		}
+	}
 
+	if (use_active) {
 		active_media_state = ast_sip_session_media_state_clone(session->active_media_state);
 		if (!active_media_state) {
 			ast_sip_session_media_state_free(pending_media_state);

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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I61313cca7fc571144338aac826091791b87b6e17
Gerrit-Change-Number: 15638
Gerrit-PatchSet: 3
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210322/904170e9/attachment.html>


More information about the asterisk-code-review mailing list