[Asterisk-code-review] pbx.c: On error, ast_add_extension2_lockopt should always free 'data' (asterisk[16])

[Sean Bright]

Sean Bright has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/15022 )


Change subject: pbx.c: On error, ast_add_extension2_lockopt should always free 'data'
......................................................................

pbx.c: On error, ast_add_extension2_lockopt should always free 'data'

In the event that the desired extension already exists,
ast_add_extension2_lockopt() will free the 'data' it is passed before
returning an error, so we should not be freeing it ourselves.

Additionally, there were two places where ast_add_extension2_lockopt()
could return an error without also freeing the 'data' pointer, so we
add that.

ASTERISK-29097 #close

Change-Id: I904707aae55169feda050a5ed7c6793b53fe6eae
---
M main/pbx.c
M res/parking/parking_bridge_features.c
M res/res_parking.c
M res/res_pjsip_config_wizard.c
4 files changed, 11 insertions(+), 4 deletions(-)



  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/22/15022/1

diff --git a/main/pbx.c b/main/pbx.c
index b520f5f..0609240 100644
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -7346,6 +7346,10 @@
 	if (ast_strlen_zero(extension)) {
 		ast_log(LOG_ERROR,"You have to be kidding-- add exten '' to context %s? Figure out a name and call me back. Action ignored.\n",
 				con->name);
+		/* We always need to deallocate 'data' on failure */
+		if (datad) {
+			datad(data);
+		}
 		return -1;
 	}
 
@@ -7401,8 +7405,14 @@
 	}
 
 	/* Be optimistic:  Build the extension structure first */
-	if (!(tmp = ast_calloc(1, length)))
+	tmp = ast_calloc(1, length);
+	if (!tmp) {
+		/* We always need to deallocate 'data' on failure */
+		if (datad) {
+			datad(data);
+		}
 		return -1;
+	}
 
 	if (ast_strlen_zero(label)) /* let's turn empty labels to a null ptr */
 		label = 0;
diff --git a/res/parking/parking_bridge_features.c b/res/parking/parking_bridge_features.c
index 7728f0d..5347e6f 100644
--- a/res/parking/parking_bridge_features.c
+++ b/res/parking/parking_bridge_features.c
@@ -661,7 +661,6 @@
 			dial_string_flat, PARK_DIAL_CONTEXT, ast_get_extension_registrar(existing_exten));
 	} else if (ast_add_extension2_nolock(park_dial_context, 1, dial_string_flat, 1, NULL, NULL,
 			"Dial", duplicate_returnexten, ast_free_ptr, BASE_REGISTRAR, NULL, 0)) {
-			ast_free(duplicate_returnexten);
 		ast_log(LOG_ERROR, "Failed to create parking redial parker extension %s@%s - Dial(%s)\n",
 			dial_string_flat, PARK_DIAL_CONTEXT, returnexten);
 	}
diff --git a/res/res_parking.c b/res/res_parking.c
index afde599..2aa19c9 100644
--- a/res/res_parking.c
+++ b/res/res_parking.c
@@ -724,7 +724,6 @@
 
 	if (ast_add_extension2_nolock(context, replace, extension, priority, NULL, NULL,
 			application, data_duplicate, ast_free_ptr, registrar, NULL, 0)) {
-		ast_free(data_duplicate);
 		return -1;
 	}
 
diff --git a/res/res_pjsip_config_wizard.c b/res/res_pjsip_config_wizard.c
index aec4b58..e61b7c5 100644
--- a/res/res_pjsip_config_wizard.c
+++ b/res/res_pjsip_config_wizard.c
@@ -475,7 +475,6 @@
 
 	if (ast_add_extension2_nolock(context, 0, exten, priority, NULL, NULL,
 			app, data, free_ptr, BASE_REGISTRAR, NULL, 0)) {
-		ast_free(data);
 		return -1;
 	}
 

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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I904707aae55169feda050a5ed7c6793b53fe6eae
Gerrit-Change-Number: 15022
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200929/b5910e6f/attachment-0001.html>