<p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4; color: #000000;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/c/asterisk/+/15322">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/asterisk/+/15322/1//COMMIT_MSG">Commit Message:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15322/1//COMMIT_MSG@7">Patch Set #1, Line 7:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">main/pbx_variables.c: Better parsing of variable name and value separator for Set command<br><br>The variable name and value of a Set command are separated by a =, but the = character<br>may appear in the variable name portion as a parameter for a function, like any ODBC_*<br>func_odbc.conf function, so better handling of quotes, parenthesis, and brackets was<br>needed.<br><br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">These lines are too long.  Need to be shorter than 72 characters.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/c/asterisk/+/15322/1/main/pbx_variables.c">File main/pbx_variables.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15322/1/main/pbx_variables.c@1131">Patch Set #1, Line 1131:</a> <code style="font-family:monospace,monospace">     char *name = NULL, *value = NULL, *mydata, *scan;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I know they were on the same line originally but we'd prefer the declarations to be separated.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/c/asterisk/+/15322/1/main/pbx_variables.c@1143">Patch Set #1, Line 1143:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  for (; *scan; scan++) {<br>          if (*scan == '(') {<br>            paren++;<br>          } else if (*scan == ')') {<br>            if (paren) {<br>              paren--;<br>            }<br>          } else if (*scan == '[') {<br>            bracket++;<br>          } else if (*scan == ']') {<br>            if (bracket) {<br>              bracket--;<br>            }<br>          } else if (*scan == '"') {<br>            quote = quote ? 0 : 1;<br>          } else if (*scan == '\\') {<br>            scan++;<br>          } else if ((*scan == '=') && !paren && !quote && !bracket) {<br>            *scan++ = '\0';<br>            value = scan;<br>            name = mydata;<br>            break;<br>          }<br>        }<br><br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I'd suggest that you move this to utils.c as maybe </p><p style="white-space: pre-wrap; word-wrap: break-word;">int ast_parse_name_value(const char *input, char **name, size_t max_name_len, char **value, size_t max_value_len).</p><p style="white-space: pre-wrap; word-wrap: break-word;">Then you can create a unit test to make sure that you have all of the bases covered (and that someone else doesn't come along and screw it up later).</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/c/asterisk/+/15322">change 15322</a>. To unsubscribe, or for help writing mail filters, 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/c/asterisk/+/15322"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 16 </div>
<div style="display:none"> Gerrit-Change-Id: I88cd6230bfd0295a6a1c0b08d3ca37c006f561e6 </div>
<div style="display:none"> Gerrit-Change-Number: 15322 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Leandro Dardini <ldardini@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Friendly Automation </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 13 Jan 2021 13:22:23 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>