<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/8038">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Benjamin Keith Ford: Looks good to me, but someone else must approve
Joshua Colp: Looks good to me, approved
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">pbx_variables.c: Misc fixes in variable substitution.<br><br>* Copy more than one character at a time when there is nothing to<br>substitute.<br><br>* Fix off by one error if a '}' or ']' is missing.<br><br>* Eliminated the requirement that the "used" parameter had to point to a<br>variable. The current callers were always declaring a variable to meet<br>the requirement and discarding the value put into that variable. Now it<br>can be NULL.<br><br>* In ast_str_substitute_variables_full() fixed using the bogus channel to<br>evaluate a function. We were not using the bogus channel we just created<br>to help evaluate a subexpression.<br><br>Change-Id: Ia83d99f4f16abe47f329eb39b6ff2013ae7c9854<br>---<br>M include/asterisk/pbx.h<br>M main/pbx_variables.c<br>2 files changed, 157 insertions(+), 90 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/include/asterisk/pbx.h b/include/asterisk/pbx.h<br>index f177b09..6dea744 100644<br>--- a/include/asterisk/pbx.h<br>+++ b/include/asterisk/pbx.h<br>@@ -1378,7 +1378,7 @@<br> * \param c Channel variables from which to extract values, and channel to pass to any dialplan functions.<br> * \param headp If no channel is specified, a channel list from which to extract variable values<br> * \param templ Variable template to expand.<br>- * \param used Number of bytes read from the template.<br>+ * \param used Number of bytes read from the template. (May be NULL)<br> */<br> void ast_str_substitute_variables_full(struct ast_str **buf, ssize_t maxlen, struct ast_channel *c, struct varshead *headp, const char *templ, size_t *used);<br> /*! @} */<br>diff --git a/main/pbx_variables.c b/main/pbx_variables.c<br>index 16ddbd0..f09540b 100644<br>--- a/main/pbx_variables.c<br>+++ b/main/pbx_variables.c<br>@@ -398,51 +398,74 @@<br> void ast_str_substitute_variables_full(struct ast_str **buf, ssize_t maxlen, struct ast_channel *c, struct varshead *headp, const char *templ, size_t *used)<br> {<br> /* Substitutes variables into buf, based on string templ */<br>- char *cp4 = NULL;<br> const char *whereweare;<br>- int orig_size = 0;<br>- int offset, offset2, isfunction;<br>- const char *nextvar, *nextexp, *nextthing;<br>- const char *vars, *vare;<br>- char *finalvars;<br>- int pos, brackets, needsub, len;<br>- struct ast_str *substr1 = ast_str_create(16), *substr2 = NULL, *substr3 = ast_str_create(16);<br>+ struct ast_str *substr1 = ast_str_create(16);<br>+ struct ast_str *substr2 = NULL;<br>+ struct ast_str *substr3 = ast_str_create(16);<br> <br> ast_str_reset(*buf);<br>+<br>+ if (!substr1 || !substr3) {<br>+ if (used) {<br>+ *used = ast_str_strlen(*buf);<br>+ }<br>+ ast_free(substr1);<br>+ ast_free(substr3);<br>+ return;<br>+ }<br>+<br> whereweare = templ;<br> while (!ast_strlen_zero(whereweare)) {<br>+ const char *nextvar = NULL;<br>+ const char *nextexp = NULL;<br>+ const char *nextthing;<br>+ const char *vars;<br>+ const char *vare;<br>+ char *finalvars;<br>+ int pos;<br>+ int brackets;<br>+ int needsub;<br>+ int len;<br>+<br> /* reset our buffer */<br> ast_str_reset(substr3);<br> <br>- /* Assume we're copying the whole remaining string */<br>- pos = strlen(whereweare);<br>- nextvar = NULL;<br>- nextexp = NULL;<br>+ /* Determine how much simply needs to be copied to the output buf. */<br> nextthing = strchr(whereweare, '$');<br> if (nextthing) {<br>+ pos = nextthing - whereweare;<br> switch (nextthing[1]) {<br> case '{':<br>+ /* Variable substitution */<br> nextvar = nextthing;<br>- pos = nextvar - whereweare;<br> break;<br> case '[':<br>+ /* Expression substitution */<br> nextexp = nextthing;<br>- pos = nextexp - whereweare;<br> break;<br> default:<br>- pos = 1;<br>+ /* '$' is not part of a substitution so include it too. */<br>+ ++pos;<br>+ break;<br> }<br>+ } else {<br>+ /* We're copying the whole remaining string */<br>+ pos = strlen(whereweare);<br> }<br> <br> if (pos) {<br> /* Copy that many bytes */<br> ast_str_append_substr(buf, maxlen, whereweare, pos);<br> <br>- templ += pos;<br> whereweare += pos;<br> }<br> <br> if (nextvar) {<br>+ int offset;<br>+ int offset2;<br>+ int isfunction;<br>+ int res;<br>+<br> /* We have a variable. Find the start and end, and determine<br> if we are going to have to recursively call ourselves on the<br> contents */<br>@@ -454,33 +477,42 @@<br> while (brackets && *vare) {<br> if ((vare[0] == '$') && (vare[1] == '{')) {<br> needsub++;<br>+ brackets++;<br>+ vare++;<br> } else if (vare[0] == '{') {<br> brackets++;<br> } else if (vare[0] == '}') {<br> brackets--;<br>- } else if ((vare[0] == '$') && (vare[1] == '['))<br>+ } else if ((vare[0] == '$') && (vare[1] == '[')) {<br> needsub++;<br>+ vare++;<br>+ }<br> vare++;<br> }<br>- if (brackets)<br>+ len = vare - vars;<br>+ if (brackets) {<br> ast_log(LOG_WARNING, "Error in extension logic (missing '}')\n");<br>- len = vare - vars - 1;<br>+ } else {<br>+ /* Don't count the closing '}' in the length. */<br>+ --len;<br>+ }<br> <br> /* Skip totally over variable string */<br>- whereweare += (len + 3);<br>+ whereweare = vare;<br> <br>- /* Store variable name (and truncate) */<br>+ /* Store variable name expression to lookup. */<br> ast_str_set_substr(&substr1, 0, vars, len);<br> ast_debug(5, "Evaluating '%s' (from '%s' len %d)\n", ast_str_buffer(substr1), vars, len);<br> <br> /* Substitute if necessary */<br> if (needsub) {<br>- size_t my_used;<br>-<br> if (!substr2) {<br> substr2 = ast_str_create(16);<br>+ if (!substr2) {<br>+ continue;<br>+ }<br> }<br>- ast_str_substitute_variables_full(&substr2, 0, c, headp, ast_str_buffer(substr1), &my_used);<br>+ ast_str_substitute_variables_full(&substr2, 0, c, headp, ast_str_buffer(substr1), NULL);<br> finalvars = ast_str_buffer(substr2);<br> } else {<br> finalvars = ast_str_buffer(substr1);<br>@@ -490,28 +522,32 @@<br> if (isfunction) {<br> /* Evaluate function */<br> if (c || !headp) {<br>- cp4 = ast_func_read2(c, finalvars, &substr3, 0) ? NULL : ast_str_buffer(substr3);<br>+ res = ast_func_read2(c, finalvars, &substr3, 0);<br> } else {<br> struct varshead old;<br>- struct ast_channel *bogus = ast_dummy_channel_alloc();<br>+ struct ast_channel *bogus;<br>+<br>+ bogus = ast_dummy_channel_alloc();<br> if (bogus) {<br>- memcpy(&old, ast_channel_varshead(bogus), sizeof(old));<br>- memcpy(ast_channel_varshead(bogus), headp, sizeof(*ast_channel_varshead(bogus)));<br>- cp4 = ast_func_read2(c, finalvars, &substr3, 0) ? NULL : ast_str_buffer(substr3);<br>+ old = *ast_channel_varshead(bogus);<br>+ *ast_channel_varshead(bogus) = *headp;<br>+ res = ast_func_read2(bogus, finalvars, &substr3, 0);<br> /* Don't deallocate the varshead that was passed in */<br>- memcpy(ast_channel_varshead(bogus), &old, sizeof(*ast_channel_varshead(bogus)));<br>+ *ast_channel_varshead(bogus) = old;<br> ast_channel_unref(bogus);<br> } else {<br>- ast_log(LOG_ERROR, "Unable to allocate bogus channel for variable substitution. Function results may be blank.\n");<br>+ ast_log(LOG_ERROR, "Unable to allocate bogus channel for function value substitution.\n");<br>+ res = -1;<br> }<br> }<br>- ast_debug(2, "Function %s result is '%s'\n", finalvars, cp4 ? cp4 : "(null)");<br>+ ast_debug(2, "Function %s result is '%s'\n",<br>+ finalvars, res ? "" : ast_str_buffer(substr3));<br> } else {<br> /* Retrieve variable value */<br> ast_str_retrieve_variable(&substr3, 0, c, headp, finalvars);<br>- cp4 = ast_str_buffer(substr3);<br>+ res = 0;<br> }<br>- if (cp4) {<br>+ if (!res) {<br> ast_str_substring(substr3, offset, offset2);<br> ast_str_append(buf, maxlen, "%s", ast_str_buffer(substr3));<br> }<br>@@ -539,24 +575,29 @@<br> }<br> vare++;<br> }<br>- if (brackets)<br>+ len = vare - vars;<br>+ if (brackets) {<br> ast_log(LOG_WARNING, "Error in extension logic (missing ']')\n");<br>- len = vare - vars - 1;<br>+ } else {<br>+ /* Don't count the closing ']' in the length. */<br>+ --len;<br>+ }<br> <br> /* Skip totally over expression */<br>- whereweare += (len + 3);<br>+ whereweare = vare;<br> <br>- /* Store variable name (and truncate) */<br>+ /* Store expression to evaluate. */<br> ast_str_set_substr(&substr1, 0, vars, len);<br> <br> /* Substitute if necessary */<br> if (needsub) {<br>- size_t my_used;<br>-<br> if (!substr2) {<br> substr2 = ast_str_create(16);<br>+ if (!substr2) {<br>+ continue;<br>+ }<br> }<br>- ast_str_substitute_variables_full(&substr2, 0, c, headp, ast_str_buffer(substr1), &my_used);<br>+ ast_str_substitute_variables_full(&substr2, 0, c, headp, ast_str_buffer(substr1), NULL);<br> finalvars = ast_str_buffer(substr2);<br> } else {<br> finalvars = ast_str_buffer(substr1);<br>@@ -568,7 +609,9 @@<br> ast_str_append(buf, maxlen, "%s", ast_str_buffer(substr3));<br> }<br> }<br>- *used = ast_str_strlen(*buf) - orig_size;<br>+ if (used) {<br>+ *used = ast_str_strlen(*buf);<br>+ }<br> ast_free(substr1);<br> ast_free(substr2);<br> ast_free(substr3);<br>@@ -576,49 +619,58 @@<br> <br> void ast_str_substitute_variables(struct ast_str **buf, ssize_t maxlen, struct ast_channel *chan, const char *templ)<br> {<br>- size_t used;<br>- ast_str_substitute_variables_full(buf, maxlen, chan, NULL, templ, &used);<br>+ ast_str_substitute_variables_full(buf, maxlen, chan, NULL, templ, NULL);<br> }<br> <br> void ast_str_substitute_variables_varshead(struct ast_str **buf, ssize_t maxlen, struct varshead *headp, const char *templ)<br> {<br>- size_t used;<br>- ast_str_substitute_variables_full(buf, maxlen, NULL, headp, templ, &used);<br>+ ast_str_substitute_variables_full(buf, maxlen, NULL, headp, templ, NULL);<br> }<br> <br> void pbx_substitute_variables_helper_full(struct ast_channel *c, struct varshead *headp, const char *cp1, char *cp2, int count, size_t *used)<br> {<br> /* Substitutes variables into cp2, based on string cp1, cp2 NO LONGER NEEDS TO BE ZEROED OUT!!!! */<br>- char *cp4 = NULL;<br>- const char *whereweare, *orig_cp2 = cp2;<br>- int length, offset, offset2, isfunction;<br>+ const char *whereweare;<br>+ const char *orig_cp2 = cp2;<br> char *workspace = NULL;<br>- char *ltmp = NULL, *var = NULL;<br>- char *nextvar, *nextexp, *nextthing;<br>- char *vars, *vare;<br>- int pos, brackets, needsub, len;<br>+ char *ltmp = NULL;<br>+ char *var = NULL;<br> <br> *cp2 = 0; /* just in case nothing ends up there */<br> whereweare = cp1;<br> while (!ast_strlen_zero(whereweare) && count) {<br>- /* Assume we're copying the whole remaining string */<br>- pos = strlen(whereweare);<br>- nextvar = NULL;<br>- nextexp = NULL;<br>+ char *nextvar = NULL;<br>+ char *nextexp = NULL;<br>+ char *nextthing;<br>+ char *vars;<br>+ char *vare;<br>+ int length;<br>+ int pos;<br>+ int brackets;<br>+ int needsub;<br>+ int len;<br>+<br>+ /* Determine how much simply needs to be copied to the output buf. */<br> nextthing = strchr(whereweare, '$');<br> if (nextthing) {<br>+ pos = nextthing - whereweare;<br> switch (nextthing[1]) {<br> case '{':<br>+ /* Variable substitution */<br> nextvar = nextthing;<br>- pos = nextvar - whereweare;<br> break;<br> case '[':<br>+ /* Expression substitution */<br> nextexp = nextthing;<br>- pos = nextexp - whereweare;<br> break;<br> default:<br>- pos = 1;<br>+ /* '$' is not part of a substitution so include it too. */<br>+ ++pos;<br>+ break;<br> }<br>+ } else {<br>+ /* We're copying the whole remaining string */<br>+ pos = strlen(whereweare);<br> }<br> <br> if (pos) {<br>@@ -636,6 +688,11 @@<br> }<br> <br> if (nextvar) {<br>+ int offset;<br>+ int offset2;<br>+ int isfunction;<br>+ char *cp4;<br>+<br> /* We have a variable. Find the start and end, and determine<br> if we are going to have to recursively call ourselves on the<br> contents */<br>@@ -647,35 +704,41 @@<br> while (brackets && *vare) {<br> if ((vare[0] == '$') && (vare[1] == '{')) {<br> needsub++;<br>+ brackets++;<br>+ vare++;<br> } else if (vare[0] == '{') {<br> brackets++;<br> } else if (vare[0] == '}') {<br> brackets--;<br>- } else if ((vare[0] == '$') && (vare[1] == '['))<br>+ } else if ((vare[0] == '$') && (vare[1] == '[')) {<br> needsub++;<br>+ vare++;<br>+ }<br> vare++;<br> }<br>- if (brackets)<br>+ len = vare - vars;<br>+ if (brackets) {<br> ast_log(LOG_WARNING, "Error in extension logic (missing '}')\n");<br>- len = vare - vars - 1;<br>+ } else {<br>+ /* Don't count the closing '}' in the length. */<br>+ --len;<br>+ }<br> <br> /* Skip totally over variable string */<br>- whereweare += (len + 3);<br>+ whereweare = vare;<br> <br> if (!var)<br> var = ast_alloca(VAR_BUF_SIZE);<br> <br>- /* Store variable name (and truncate) */<br>+ /* Store variable name expression to lookup (and truncate). */<br> ast_copy_string(var, vars, len + 1);<br> <br> /* Substitute if necessary */<br> if (needsub) {<br>- size_t my_used;<br>-<br> if (!ltmp) {<br> ltmp = ast_alloca(VAR_BUF_SIZE);<br> }<br>- pbx_substitute_variables_helper_full(c, headp, var, ltmp, VAR_BUF_SIZE - 1, &my_used);<br>+ pbx_substitute_variables_helper_full(c, headp, var, ltmp, VAR_BUF_SIZE - 1, NULL);<br> vars = ltmp;<br> } else {<br> vars = var;<br>@@ -693,16 +756,19 @@<br> cp4 = ast_func_read(c, vars, workspace, VAR_BUF_SIZE) ? NULL : workspace;<br> else {<br> struct varshead old;<br>- struct ast_channel *c = ast_dummy_channel_alloc();<br>- if (c) {<br>- memcpy(&old, ast_channel_varshead(c), sizeof(old));<br>- memcpy(ast_channel_varshead(c), headp, sizeof(*ast_channel_varshead(c)));<br>- cp4 = ast_func_read(c, vars, workspace, VAR_BUF_SIZE) ? NULL : workspace;<br>+ struct ast_channel *bogus;<br>+<br>+ bogus = ast_dummy_channel_alloc();<br>+ if (bogus) {<br>+ old = *ast_channel_varshead(bogus);<br>+ *ast_channel_varshead(bogus) = *headp;<br>+ cp4 = ast_func_read(bogus, vars, workspace, VAR_BUF_SIZE) ? NULL : workspace;<br> /* Don't deallocate the varshead that was passed in */<br>- memcpy(ast_channel_varshead(c), &old, sizeof(*ast_channel_varshead(c)));<br>- c = ast_channel_unref(c);<br>+ *ast_channel_varshead(bogus) = old;<br>+ ast_channel_unref(bogus);<br> } else {<br>- ast_log(LOG_ERROR, "Unable to allocate bogus channel for variable substitution. Function results may be blank.\n");<br>+ ast_log(LOG_ERROR, "Unable to allocate bogus channel for function value substitution.\n");<br>+ cp4 = NULL;<br> }<br> }<br> ast_debug(2, "Function %s result is '%s'\n", vars, cp4 ? cp4 : "(null)");<br>@@ -745,34 +811,35 @@<br> }<br> vare++;<br> }<br>- if (brackets)<br>+ len = vare - vars;<br>+ if (brackets) {<br> ast_log(LOG_WARNING, "Error in extension logic (missing ']')\n");<br>- len = vare - vars - 1;<br>+ } else {<br>+ /* Don't count the closing ']' in the length. */<br>+ --len;<br>+ }<br> <br> /* Skip totally over expression */<br>- whereweare += (len + 3);<br>+ whereweare = vare;<br> <br> if (!var)<br> var = ast_alloca(VAR_BUF_SIZE);<br> <br>- /* Store variable name (and truncate) */<br>+ /* Store expression to evaluate (and truncate). */<br> ast_copy_string(var, vars, len + 1);<br> <br> /* Substitute if necessary */<br> if (needsub) {<br>- size_t my_used;<br>-<br> if (!ltmp) {<br> ltmp = ast_alloca(VAR_BUF_SIZE);<br> }<br>- pbx_substitute_variables_helper_full(c, headp, var, ltmp, VAR_BUF_SIZE - 1, &my_used);<br>+ pbx_substitute_variables_helper_full(c, headp, var, ltmp, VAR_BUF_SIZE - 1, NULL);<br> vars = ltmp;<br> } else {<br> vars = var;<br> }<br> <br> length = ast_expr(vars, cp2, count, c);<br>-<br> if (length) {<br> ast_debug(1, "Expression result is '%s'\n", cp2);<br> count -= length;<br>@@ -781,19 +848,19 @@<br> }<br> }<br> }<br>- *used = cp2 - orig_cp2;<br>+ if (used) {<br>+ *used = cp2 - orig_cp2;<br>+ }<br> }<br> <br> void pbx_substitute_variables_helper(struct ast_channel *c, const char *cp1, char *cp2, int count)<br> {<br>- size_t used;<br>- pbx_substitute_variables_helper_full(c, (c) ? ast_channel_varshead(c) : NULL, cp1, cp2, count, &used);<br>+ pbx_substitute_variables_helper_full(c, (c) ? ast_channel_varshead(c) : NULL, cp1, cp2, count, NULL);<br> }<br> <br> void pbx_substitute_variables_varshead(struct varshead *headp, const char *cp1, char *cp2, int count)<br> {<br>- size_t used;<br>- pbx_substitute_variables_helper_full(NULL, headp, cp1, cp2, count, &used);<br>+ pbx_substitute_variables_helper_full(NULL, headp, cp1, cp2, count, NULL);<br> }<br> <br> /*! \brief CLI support for listing global variables in a parseable way */<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/8038">change 8038</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/8038"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ia83d99f4f16abe47f329eb39b6ff2013ae7c9854 </div>
<div style="display:none"> Gerrit-Change-Number: 8038 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Benjamin Keith Ford <bford@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>