[asterisk-commits] SDP: Misc cleanups (Mostly memory leaks) (asterisk[master])

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Mon May 1 14:19:34 CDT 2017


Jenkins2 has submitted this change and it was merged. ( https://gerrit.asterisk.org/5551 )

Change subject: SDP: Misc cleanups (Mostly memory leaks)
......................................................................


SDP: Misc cleanups (Mostly memory leaks)

Change-Id: I74431b385da333f2c5f5a6d7c55e70b69a4f05d2
---
M include/asterisk/sdp.h
M main/sdp.c
M main/sdp_state.c
M main/sdp_translator.c
M res/res_sdp_translator_pjmedia.c
5 files changed, 60 insertions(+), 27 deletions(-)

Approvals:
  Mark Michelson: Looks good to me, but someone else must approve
  Jenkins2: Approved for Submit
  Joshua Colp: Looks good to me, approved



diff --git a/include/asterisk/sdp.h b/include/asterisk/sdp.h
index d5bf914..06470c4 100644
--- a/include/asterisk/sdp.h
+++ b/include/asterisk/sdp.h
@@ -535,8 +535,8 @@
  * \param format Format
  * \param code from AST_RTP list
  *
- * \retval non-NULL Success
- * \retval NULL Failure
+ * \retval 0 Success
+ * \retval non-0 Failure
  *
  * \since 15
  */
diff --git a/main/sdp.c b/main/sdp.c
index dc6afe7..62acdd3 100644
--- a/main/sdp.c
+++ b/main/sdp.c
@@ -435,19 +435,25 @@
 	int rtp_code)
 {
 	struct ast_str *fmtp0 = ast_str_alloca(256);
+	struct ast_sdp_a_line *a_line;
 	char *tmp;
 
 	ast_format_generate_sdp_fmtp(format, rtp_code, &fmtp0);
 	if (ast_str_strlen(fmtp0) == 0) {
-		return -1;
+		/* Format doesn't have fmtp attributes */
+		return 0;
 	}
 
 	tmp = ast_str_buffer(fmtp0) + ast_str_strlen(fmtp0) - 1;
-		/* remove any carriage return line feeds */
+	/* remove any carriage return line feeds */
 	while (*tmp == '\r' || *tmp == '\n') --tmp;
 	*++tmp = '\0';
 
-	/* ast...generate gives us everything, just need value */
+	/*
+	 * ast...generate gives us everything, just need value
+	 *
+	 * It can also give multiple fmtp attribute lines. (silk does)
+	 */
 	tmp = strchr(ast_str_buffer(fmtp0), ':');
 	if (tmp && tmp[1] != '\0') {
 		tmp++;
@@ -455,7 +461,10 @@
 		tmp = ast_str_buffer(fmtp0);
 	}
 
-	ast_sdp_m_add_a(m_line, ast_sdp_a_alloc("fmtp", tmp));
+	a_line = ast_sdp_a_alloc("fmtp", tmp);
+	if (!a_line || ast_sdp_m_add_a(m_line, a_line)) {
+		return -1;
+	}
 
 	return 0;
 }
@@ -495,10 +504,8 @@
 int ast_sdp_m_add_format(struct ast_sdp_m_line *m_line, const struct ast_sdp_options *options,
 	int rtp_code, int asterisk_format, const struct ast_format *format, int code)
 {
-	sdp_m_add_rtpmap(m_line, options, rtp_code, asterisk_format, format, code);
-	sdp_m_add_fmtp(m_line, format, rtp_code);
-
-	return 0;
+	return sdp_m_add_rtpmap(m_line, options, rtp_code, asterisk_format, format, code)
+		|| sdp_m_add_fmtp(m_line, format, rtp_code) ? -1 : 0;
 }
 
 static struct ast_sdp_a_line *sdp_find_attribute_common(const struct ast_sdp_a_lines *a_lines,
diff --git a/main/sdp_state.c b/main/sdp_state.c
index 5aee567..8534501 100644
--- a/main/sdp_state.c
+++ b/main/sdp_state.c
@@ -30,7 +30,7 @@
 #include "asterisk/codec.h"
 #include "asterisk/udptl.h"
 
-#include "../include/asterisk/sdp.h"
+#include "asterisk/sdp.h"
 #include "asterisk/stream.h"
 
 #include "sdp_private.h"
@@ -171,7 +171,7 @@
 		return NULL;
 	}
 
-	ast_rtp_instance_set_prop(rtp, AST_RTP_PROPERTY_RTCP, 1);
+	ast_rtp_instance_set_prop(rtp, AST_RTP_PROPERTY_RTCP, AST_RTP_INSTANCE_RTCP_STANDARD);
 	ast_rtp_instance_set_prop(rtp, AST_RTP_PROPERTY_NAT, options->rtp_symmetric);
 
 	if (options->ice == AST_SDP_ICE_DISABLED && (ice = ast_rtp_instance_get_ice(rtp))) {
@@ -252,8 +252,7 @@
 		return NULL;
 	}
 
-	if (AST_VECTOR_INIT(&capabilities->streams,
-			ast_stream_topology_get_count(topology))) {
+	if (AST_VECTOR_INIT(&capabilities->streams, ast_stream_topology_get_count(topology))) {
 		sdp_state_capabilities_free(capabilities);
 		return NULL;
 	}
@@ -264,17 +263,18 @@
 
 		state_stream = ast_calloc(1, sizeof(*state_stream));
 		if (!state_stream) {
+			sdp_state_capabilities_free(capabilities);
 			return NULL;
 		}
 
 		state_stream->type = ast_stream_get_type(ast_stream_topology_get_stream(topology, i));
-
 		switch (state_stream->type) {
 		case AST_MEDIA_TYPE_AUDIO:
 		case AST_MEDIA_TYPE_VIDEO:
 			state_stream->instance = create_rtp(options, state_stream->type);
 			if (!state_stream->instance) {
 				sdp_state_stream_free(state_stream);
+				sdp_state_capabilities_free(capabilities);
 				return NULL;
 			}
 			break;
@@ -282,16 +282,24 @@
 			state_stream->udptl = create_udptl(options);
 			if (!state_stream->udptl) {
 				sdp_state_stream_free(state_stream);
+				sdp_state_capabilities_free(capabilities);
 				return NULL;
 			}
 			break;
 		case AST_MEDIA_TYPE_UNKNOWN:
 		case AST_MEDIA_TYPE_TEXT:
 		case AST_MEDIA_TYPE_END:
-			break;
+			ast_assert(0);
+			sdp_state_stream_free(state_stream);
+			sdp_state_capabilities_free(capabilities);
+			return NULL;
 		}
 
-		AST_VECTOR_APPEND(&capabilities->streams, state_stream);
+		if (AST_VECTOR_APPEND(&capabilities->streams, state_stream)) {
+			sdp_state_stream_free(state_stream);
+			sdp_state_capabilities_free(capabilities);
+			return NULL;
+		}
 	}
 
 	return capabilities;
@@ -632,7 +640,9 @@
 		goto fail;
 	}
 
-	AST_VECTOR_INIT(&joint_capabilities->streams, AST_VECTOR_SIZE(&current->streams));
+	if (AST_VECTOR_INIT(&joint_capabilities->streams, AST_VECTOR_SIZE(&current->streams))) {
+		goto fail;
+	}
 	ast_sockaddr_copy(&joint_capabilities->connection_address, &current->connection_address);
 	topology = current->topology;
 
@@ -654,11 +664,11 @@
 		new_stream_type = ast_stream_get_type(new_stream);
 
 		current_index = get_corresponding_index(topology, new_stream_type, media_indices);
-
 		if (current_index >= 0) {
 			current_stream = ast_stream_topology_get_stream(topology, current_index);
 			joint_stream = merge_streams(current_stream, new_stream);
 			if (!joint_stream) {
+				sdp_state_stream_free(joint_state_stream);
 				goto fail;
 			}
 
@@ -692,6 +702,7 @@
 			 */
 			joint_stream = ast_stream_clone(new_stream);
 			if (!joint_stream) {
+				sdp_state_stream_free(joint_state_stream);
 				goto fail;
 			}
 
@@ -700,12 +711,16 @@
 			case AST_MEDIA_TYPE_VIDEO:
 				joint_state_stream->instance = create_rtp(options, new_stream_type);
 				if (!joint_state_stream->instance) {
+					ast_stream_free(joint_stream);
+					sdp_state_stream_free(joint_state_stream);
 					goto fail;
 				}
 				break;
 			case AST_MEDIA_TYPE_IMAGE:
 				joint_state_stream->udptl = create_udptl(options);
 				if (!joint_state_stream->udptl) {
+					ast_stream_free(joint_stream);
+					sdp_state_stream_free(joint_state_stream);
 					goto fail;
 				}
 				break;
@@ -723,12 +738,20 @@
 			 */
 			joint_stream = ast_stream_alloc("dummy", new_stream_type);
 			if (!joint_stream) {
+				sdp_state_stream_free(joint_state_stream);
 				goto fail;
 			}
 		}
 
-		ast_stream_topology_append_stream(joint_capabilities->topology, joint_stream);
-		AST_VECTOR_APPEND(&joint_capabilities->streams, joint_state_stream);
+		if (ast_stream_topology_append_stream(joint_capabilities->topology, joint_stream) < 0) {
+			ast_stream_free(joint_stream);
+			sdp_state_stream_free(joint_state_stream);
+			goto fail;
+		}
+		if (AST_VECTOR_APPEND(&joint_capabilities->streams, joint_state_stream)) {
+			sdp_state_stream_free(joint_state_stream);
+			goto fail;
+		}
 	}
 
 	return joint_capabilities;
@@ -974,8 +997,7 @@
  * \retval -1 Failure
  * \retval 0 Success
  */
-static int merge_sdps(struct ast_sdp_state *sdp_state,
-	const struct ast_sdp *remote_sdp)
+static int merge_sdps(struct ast_sdp_state *sdp_state, const struct ast_sdp *remote_sdp)
 {
 	struct sdp_state_capabilities *joint_capabilities;
 	int i;
@@ -1073,7 +1095,7 @@
 		return -1;
 	}
 	ast_sdp_state_set_remote_sdp(sdp_state, sdp);
-
+	ast_sdp_free(sdp);
 	return 0;
 }
 
@@ -1254,13 +1276,13 @@
 
 			rtp_code = ast_rtp_codecs_payload_code(
 				ast_rtp_instance_get_codecs(rtp), 0, NULL, i);
-
 			if (rtp_code == -1) {
 				continue;
 			}
 
 			if (ast_sdp_m_add_format(m_line, options, rtp_code, 0, NULL, i)) {
-				continue;
+				ast_sdp_m_free(m_line);
+				return -1;
 			}
 
 			if (i == AST_RTP_DTMF) {
@@ -1276,6 +1298,7 @@
 	}
 
 	if (ast_sdp_m_get_a_count(m_line) == 0) {
+		ast_sdp_m_free(m_line);
 		return 0;
 	}
 
diff --git a/main/sdp_translator.c b/main/sdp_translator.c
index abd0f62..2426b10 100644
--- a/main/sdp_translator.c
+++ b/main/sdp_translator.c
@@ -84,6 +84,9 @@
 
 void ast_sdp_translator_free(struct ast_sdp_translator *translator)
 {
+	if (!translator) {
+		return;
+	}
 	translator->ops->translator_free(translator->translator_priv);
 	ast_free(translator);
 }
diff --git a/res/res_sdp_translator_pjmedia.c b/res/res_sdp_translator_pjmedia.c
index 8eab716..6ad1e9b 100644
--- a/res/res_sdp_translator_pjmedia.c
+++ b/res/res_sdp_translator_pjmedia.c
@@ -26,7 +26,7 @@
 #include "asterisk/test.h"
 #include "asterisk/module.h"
 
-#include "../include/asterisk/sdp.h"
+#include "asterisk/sdp.h"
 #ifdef HAVE_PJPROJECT
 #include <pjlib.h>
 #include <pjmedia.h>

-- 
To view, visit https://gerrit.asterisk.org/5551
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I74431b385da333f2c5f5a6d7c55e70b69a4f05d2
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>



More information about the asterisk-commits mailing list