[Asterisk-code-review] app record: Don't set RECORD STATUS chan var until file is ... (asterisk[15])

[Jenkins2]

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

Change subject: app_record:  Don't set RECORD_STATUS chan var until file is closed
......................................................................

app_record:  Don't set RECORD_STATUS chan var until file is closed

We've been calling pbx_builtin_setvar_helper to set the
RECORD_STATUS variable before actually closing the recorded file.
If a client is watching VarSet events and tries to do something with
the file when a RECORD_STATUS event is seen, they might attempt to
do so while the file it's still open.

We now delay calling pbx_builtin_setvar_helper until after we close
the file.

ASTERISK-27423

Change-Id: I7fe9de99953e46b4bafa2b38cf151fe8f6488254
---
M apps/app_record.c
1 file changed, 39 insertions(+), 17 deletions(-)

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



diff --git a/apps/app_record.c b/apps/app_record.c
index 8c3a577..b9014fe 100644
--- a/apps/app_record.c
+++ b/apps/app_record.c
@@ -137,6 +137,12 @@
 	OPTION_NO_TRUNCATE = (1 << 9),
 };
 
+enum dtmf_response {
+	RESPONSE_NO_MATCH = 0,
+	RESPONSE_OPERATOR,
+	RESPONSE_DTMF,
+};
+
 AST_APP_OPTIONS(app_opts,{
 	AST_APP_OPTION('a', OPTION_APPEND),
 	AST_APP_OPTION('k', OPTION_KEEP),
@@ -160,24 +166,22 @@
  * \param dtmf_integer the integer value of the DTMF key received
  * \param terminator key currently set to be pressed for normal termination
  *
- * \retval 0 do not exit
- * \retval -1 do exit
+ * \returns One of enum dtmf_response
  */
-static int record_dtmf_response(struct ast_channel *chan, struct ast_flags *flags, int dtmf_integer, int terminator)
+static enum dtmf_response record_dtmf_response(struct ast_channel *chan,
+	struct ast_flags *flags, int dtmf_integer, int terminator)
 {
 	if ((dtmf_integer == OPERATOR_KEY) &&
 		(ast_test_flag(flags, OPTION_OPERATOR_EXIT))) {
-		pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "OPERATOR");
-		return -1;
+		return RESPONSE_OPERATOR;
 	}
 
 	if ((dtmf_integer == terminator) ||
 		(ast_test_flag(flags, OPTION_ANY_TERMINATE))) {
-		pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "DTMF");
-		return -1;
+		return RESPONSE_DTMF;
 	}
 
-	return 0;
+	return RESPONSE_NO_MATCH;
 }
 
 static int create_destination_directory(const char *path)
@@ -246,6 +250,7 @@
 	);
 	int ms;
 	struct timeval start;
+	const char *status_response = "ERROR";
 
 	/* The next few lines of code parse out the filename and header from the input string */
 	if (ast_strlen_zero(data)) { /* no data implies no filename or anything is present */
@@ -343,7 +348,7 @@
 
 	if (res) {
 		ast_log(LOG_WARNING, "Could not answer channel '%s'\n", ast_channel_name(chan));
-		pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "ERROR");
+		status_response = "ERROR";
 		goto out;
 	}
 
@@ -379,7 +384,7 @@
 
 	if (create_destination_directory(tmp)) {
 		ast_log(LOG_WARNING, "Could not create directory for file %s\n", args.filename);
-		pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "ERROR");
+		status_response = "ERROR";
 		goto out;
 	}
 
@@ -388,7 +393,7 @@
 
 	if (!s) {
 		ast_log(LOG_WARNING, "Could not create file %s\n", args.filename);
-		pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "ERROR");
+		status_response = "ERROR";
 		goto out;
 	}
 
@@ -423,7 +428,7 @@
 			if (res) {
 				ast_log(LOG_WARNING, "Problem writing frame\n");
 				ast_frfree(f);
-				pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "ERROR");
+				status_response = "ERROR";
 				break;
 			}
 
@@ -439,7 +444,7 @@
 					/* Ended happily with silence */
 					ast_frfree(f);
 					gotsilence = 1;
-					pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "SILENCE");
+					status_response = "SILENCE";
 					break;
 				}
 			}
@@ -448,12 +453,26 @@
 
 			if (res) {
 				ast_log(LOG_WARNING, "Problem writing frame\n");
-				pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "ERROR");
+				status_response = "ERROR";
 				ast_frfree(f);
 				break;
 			}
 		} else if (f->frametype == AST_FRAME_DTMF) {
-			if (record_dtmf_response(chan, &flags, f->subclass.integer, terminator)) {
+			enum dtmf_response rc =
+				record_dtmf_response(chan, &flags, f->subclass.integer, terminator);
+			switch(rc) 	{
+			case RESPONSE_NO_MATCH:
+				break;
+			case RESPONSE_OPERATOR:
+				status_response = "OPERATOR";
+				ast_debug(1, "Got OPERATOR\n");
+				break;
+			case RESPONSE_DTMF:
+				status_response = "DTMF";
+				ast_debug(1, "Got DTMF\n");
+				break;
+			}
+			if (rc != RESPONSE_NO_MATCH) {
 				ast_frfree(f);
 				break;
 			}
@@ -463,13 +482,13 @@
 
 	if (maxduration > 0 && !ms) {
 		gottimeout = 1;
-		pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "TIMEOUT");
+		status_response = "TIMEOUT";
 	}
 
 	if (!f) {
 		ast_debug(1, "Got hangup\n");
 		res = -1;
-		pbx_builtin_setvar_helper(chan, "RECORD_STATUS", "HANGUP");
+		status_response = "HANGUP";
 		if (!ast_test_flag(&flags, OPTION_KEEP)) {
 			ast_filedelete(args.filename, NULL);
 		}
@@ -504,6 +523,9 @@
 	if (sildet) {
 		ast_dsp_free(sildet);
 	}
+
+	pbx_builtin_setvar_helper(chan, "RECORD_STATUS", status_response);
+
 	return res;
 }
 

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

Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: merged
Gerrit-Change-Id: I7fe9de99953e46b4bafa2b38cf151fe8f6488254
Gerrit-Change-Number: 7229
Gerrit-PatchSet: 2
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20171116/bee38011/attachment.html>