[Asterisk-code-review] app_voicemail: Fix pollmailboxes (asterisk[16.13])

[Joshua Colp]

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

Change subject: app_voicemail: Fix pollmailboxes
......................................................................

app_voicemail: Fix pollmailboxes

The name of the voicemail context was overwriting the name of the
subscribed mailbox. Fix by simplifying how we create the MWI
subscription.

ASTERISK-29029 #close

Change-Id: Ie8a7db6a0b68f3995b0846bbb733a21909ba44e5
---
M apps/app_voicemail.c
1 file changed, 23 insertions(+), 49 deletions(-)

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



diff --git a/apps/app_voicemail.c b/apps/app_voicemail.c
index ada7382..0d89e5c 100644
--- a/apps/app_voicemail.c
+++ b/apps/app_voicemail.c
@@ -1032,23 +1032,9 @@
 	int old_new;
 	int old_old;
 	char *uniqueid;
-	char mailbox[0];
+	char *mailbox;
 };
 
-struct mwi_sub_task {
-	const char *mailbox;
-	const char *context;
-	const char *uniqueid;
-};
-
-static void mwi_sub_task_dtor(struct mwi_sub_task *mwist)
-{
-	ast_free((void *) mwist->mailbox);
-	ast_free((void *) mwist->context);
-	ast_free((void *) mwist->uniqueid);
-	ast_free(mwist);
-}
-
 static struct ast_taskprocessor *mwi_subscription_tps;
 
 static AST_RWLIST_HEAD_STATIC(mwi_subs, mwi_sub);
@@ -13271,6 +13257,7 @@
 static void mwi_sub_destroy(struct mwi_sub *mwi_sub)
 {
 	ast_free(mwi_sub->uniqueid);
+	ast_free(mwi_sub->mailbox);
 	ast_free(mwi_sub);
 }
 
@@ -13349,35 +13336,12 @@
 
 static int handle_subscribe(void *datap)
 {
-	unsigned int len;
-	struct mwi_sub *mwi_sub;
-	struct mwi_sub_task *p = datap;
-	size_t context_len;
-
-	len = sizeof(*mwi_sub) + 1;
-	if (!ast_strlen_zero(p->mailbox))
-		len += strlen(p->mailbox);
-
-	context_len = strlen(p->context) + 1; /* Allow for seperator */
-	if (!ast_strlen_zero(p->context))
-		len += context_len;
-
-	if (!(mwi_sub = ast_calloc(1, len)))
-		return -1;
-
-	mwi_sub->uniqueid = ast_strdup(p->uniqueid);
-	if (!ast_strlen_zero(p->mailbox))
-		strcpy(mwi_sub->mailbox, p->mailbox);
-
-	if (!ast_strlen_zero(p->context)) {
-		strcat(mwi_sub->mailbox, "@");
-		ast_copy_string(mwi_sub->mailbox, p->context, context_len);
-	}
+	struct mwi_sub *mwi_sub = datap;
 
 	AST_RWLIST_WRLOCK(&mwi_subs);
 	AST_RWLIST_INSERT_TAIL(&mwi_subs, mwi_sub, entry);
 	AST_RWLIST_UNLOCK(&mwi_subs);
-	mwi_sub_task_dtor(p);
+
 	poll_subscribed_mailbox(mwi_sub);
 	return 0;
 }
@@ -13398,29 +13362,39 @@
 
 static void mwi_sub_event_cb(struct stasis_subscription_change *change)
 {
-	struct mwi_sub_task *mwist;
+	struct mwi_sub *mwi_sub;
 	const char *topic;
 	char *context;
 	char *mailbox;
 
-	mwist = ast_calloc(1, (sizeof(*mwist)));
-	if (!mwist) {
+	mwi_sub = ast_calloc(1, sizeof(*mwi_sub));
+	if (!mwi_sub) {
 		return;
 	}
 
 	/* The topic name is prefixed with "mwi:all/" as this is a pool topic */
 	topic = stasis_topic_name(change->topic) + 8;
 	if (separate_mailbox(ast_strdupa(topic), &mailbox, &context)) {
-		ast_free(mwist);
+		mwi_sub_destroy(mwi_sub);
 		return;
 	}
 
-	mwist->mailbox = ast_strdup(mailbox);
-	mwist->context = ast_strdup(context);
-	mwist->uniqueid = ast_strdup(change->uniqueid);
+	/* separate_mailbox() guarantees a non-NULL, non-empty mailbox and context */
+	if (ast_asprintf(&mwi_sub->mailbox, "%s@%s", mailbox, context) < 0) {
+		mwi_sub_destroy(mwi_sub);
+		return;
+	}
 
-	if (ast_taskprocessor_push(mwi_subscription_tps, handle_subscribe, mwist) < 0) {
-		mwi_sub_task_dtor(mwist);
+	/* The stasis subscription uniqueid will never be NULL */
+	mwi_sub->uniqueid = ast_strdup(change->uniqueid);
+	if (!mwi_sub->uniqueid) {
+		mwi_sub_destroy(mwi_sub);
+		return;
+	}
+
+	if (ast_taskprocessor_push(mwi_subscription_tps, handle_subscribe, mwi_sub) < 0) {
+		mwi_sub_destroy(mwi_sub);
+		return;
 	}
 }
 

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

Gerrit-Project: asterisk
Gerrit-Branch: 16.13
Gerrit-Change-Id: Ie8a7db6a0b68f3995b0846bbb733a21909ba44e5
Gerrit-Change-Number: 14862
Gerrit-PatchSet: 2
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200831/543bf6aa/attachment-0001.html>