[Asterisk-code-review] main/pbx_variables.c: Better parsing of variable name and value separ... (asterisk[16])

[George Joseph]

George Joseph has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/15322 )

Change subject: main/pbx_variables.c: Better parsing of variable name and value separator for Set command
......................................................................


Patch Set 1: Code-Review-1

(3 comments)

https://gerrit.asterisk.org/c/asterisk/+/15322/1//COMMIT_MSG 
Commit Message:

https://gerrit.asterisk.org/c/asterisk/+/15322/1//COMMIT_MSG@7 
PS1, Line 7: main/pbx_variables.c: Better parsing of variable name and value separator for Set command
           : 
           : The variable name and value of a Set command are separated by a =, but the = character
           : may appear in the variable name portion as a parameter for a function, like any ODBC_*
           : func_odbc.conf function, so better handling of quotes, parenthesis, and brackets was
           : needed.
           : 
These lines are too long.  Need to be shorter than 72 characters.


https://gerrit.asterisk.org/c/asterisk/+/15322/1/main/pbx_variables.c 
File main/pbx_variables.c:

https://gerrit.asterisk.org/c/asterisk/+/15322/1/main/pbx_variables.c@1131 
PS1, Line 1131: 	char *name = NULL, *value = NULL, *mydata, *scan;
I know they were on the same line originally but we'd prefer the declarations to be separated.


https://gerrit.asterisk.org/c/asterisk/+/15322/1/main/pbx_variables.c@1143 
PS1, Line 1143: 	for (; *scan; scan++) {
              :           if (*scan == '(') {
              :             paren++;
              :           } else if (*scan == ')') {
              :             if (paren) {
              :               paren--;
              :             }
              :           } else if (*scan == '[') {
              :             bracket++;
              :           } else if (*scan == ']') {
              :             if (bracket) {
              :               bracket--;
              :             }
              :           } else if (*scan == '"') {
              :             quote = quote ? 0 : 1;
              :           } else if (*scan == '\\') {
              :             scan++;
              :           } else if ((*scan == '=') && !paren && !quote && !bracket) {
              :             *scan++ = '\0';
              :             value = scan;
              :             name = mydata;
              :             break;
              :           }
              :         }
              : 
I'd suggest that you move this to utils.c as maybe 

int ast_parse_name_value(const char *input, char **name, size_t max_name_len, char **value, size_t max_value_len).

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).



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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I88cd6230bfd0295a6a1c0b08d3ca37c006f561e6
Gerrit-Change-Number: 15322
Gerrit-PatchSet: 1
Gerrit-Owner: Leandro Dardini <ldardini at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Comment-Date: Wed, 13 Jan 2021 13:22:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210113/87b6f51f/attachment-0001.html>