[Asterisk-code-review] T.140: Fix format ref and memory leaks. (asterisk[14])

[George Joseph]

George Joseph has submitted this change and it was merged. ( https://gerrit.asterisk.org/4799 )

Change subject: T.140: Fix format ref and memory leaks.
......................................................................


T.140: Fix format ref and memory leaks.

* channel.c:ast_sendtext(): Fix T.140 SendText memory leak.

* format_compatibility.c: T.140 RED and T.140 were swapped.

* res_rtp_asterisk.c:rtp_red_init(): Fix ast_format_t140_red ref leak.

* res_rtp_asterisk.c:rtp_red_init(): Fix data race after starting periodic
scheduled red_write().

* res_rtp_asterisk.c: Some other minor misc tweaks.

Change-Id: Ifa27a2e0f8a966b1cf628607c86fc4374b0b88cb
---
M main/channel.c
M main/format_compatibility.c
M res/res_rtp_asterisk.c
3 files changed, 16 insertions(+), 14 deletions(-)

Approvals:
  George Joseph: Looks good to me, approved
  Anonymous Coward #1000019: Verified
  Joshua Colp: Looks good to me, but someone else must approve
  Corey Farrell: Looks good to me, but someone else must approve



diff --git a/main/channel.c b/main/channel.c
index 061a445..63838d7 100644
--- a/main/channel.c
+++ b/main/channel.c
@@ -4889,16 +4889,18 @@
 	if (ast_channel_tech(chan)->write_text && (ast_format_cap_has_type(ast_channel_nativeformats(chan), AST_MEDIA_TYPE_TEXT))) {
 		struct ast_frame f;
 
+		memset(&f, 0, sizeof(f));
 		f.frametype = AST_FRAME_TEXT;
 		f.src = "DIALPLAN";
 		f.mallocd = AST_MALLOCD_DATA;
 		f.datalen = strlen(text);
 		f.data.ptr = ast_strdup(text);
-		f.offset = 0;
-		f.seqno = 0;
-
 		f.subclass.format = ast_format_t140;
-		res = ast_channel_tech(chan)->write_text(chan, &f);
+
+		if (f.data.ptr) {
+			res = ast_channel_tech(chan)->write_text(chan, &f);
+			ast_frfree(&f);
+		}
 	} else if (ast_channel_tech(chan)->send_text) {
 		res = ast_channel_tech(chan)->send_text(chan, text);
 	}
diff --git a/main/format_compatibility.c b/main/format_compatibility.c
index 9bf263b..0768b67 100644
--- a/main/format_compatibility.c
+++ b/main/format_compatibility.c
@@ -264,10 +264,10 @@
 
 	/*! T.140 RED Text format RFC 4103 */
 	case AST_FORMAT_T140_RED:
-		return ast_format_t140;
+		return ast_format_t140_red;
 	/*! T.140 Text format - ITU T.140, RFC 4103 */
 	case AST_FORMAT_T140:
-		return ast_format_t140_red;
+		return ast_format_t140;
 	}
 	return NULL;
 }
diff --git a/res/res_rtp_asterisk.c b/res/res_rtp_asterisk.c
index 947f4a6..2ab491f 100644
--- a/res/res_rtp_asterisk.c
+++ b/res/res_rtp_asterisk.c
@@ -232,6 +232,7 @@
 /*! \brief RTP session description */
 struct ast_rtp {
 	int s;
+	/*! \note The f.subclass.format holds a ref. */
 	struct ast_frame f;
 	unsigned char rawdata[8192 + AST_FRIENDLY_OFFSET];
 	unsigned int ssrc;		/*!< Synchronization source, RFC 3550, page 10. */
@@ -2767,6 +2768,7 @@
 	if (rtp->red) {
 		AST_SCHED_DEL(rtp->sched, rtp->red->schedid);
 		ast_free(rtp->red);
+		rtp->red = NULL;
 	}
 
 #ifdef HAVE_PJPROJECT
@@ -3487,7 +3489,8 @@
 	return 0;
 }
 
-static struct ast_frame *red_t140_to_red(struct rtp_red *red) {
+static struct ast_frame *red_t140_to_red(struct rtp_red *red)
+{
 	unsigned char *data = red->t140red.data.ptr;
 	int len = 0;
 	int i;
@@ -5061,22 +5064,21 @@
 	struct ast_rtp *rtp = ast_rtp_instance_get_data(instance);
 	int x;
 
-	if (!(rtp->red = ast_calloc(1, sizeof(*rtp->red)))) {
+	rtp->red = ast_calloc(1, sizeof(*rtp->red));
+	if (!rtp->red) {
 		return -1;
 	}
 
 	rtp->red->t140.frametype = AST_FRAME_TEXT;
-	ao2_replace(rtp->red->t140.subclass.format, ast_format_t140_red);
+	rtp->red->t140.subclass.format = ast_format_t140_red;
 	rtp->red->t140.data.ptr = &rtp->red->buf_data;
 
-	rtp->red->t140.ts = 0;
 	rtp->red->t140red = rtp->red->t140;
 	rtp->red->t140red.data.ptr = &rtp->red->t140red_data;
-	rtp->red->t140red.datalen = 0;
+
 	rtp->red->ti = buffer_time;
 	rtp->red->num_gen = generations;
 	rtp->red->hdrlen = generations * 4 + 1;
-	rtp->red->prev_ts = 0;
 
 	for (x = 0; x < generations; x++) {
 		rtp->red->pt[x] = payloads[x];
@@ -5085,8 +5087,6 @@
 	}
 	rtp->red->t140red_data[x*4] = rtp->red->pt[x] = payloads[x]; /* primary pt */
 	rtp->red->schedid = ast_sched_add(rtp->sched, generations, red_write, instance);
-
-	rtp->red->t140.datalen = 0;
 
 	return 0;
 }

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ifa27a2e0f8a966b1cf628607c86fc4374b0b88cb
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 14
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Corey Farrell <git at cfware.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>