[Home]

Summary:ASTERISK-04441: [patch] upgrade to check_expr to parse expressions, and give syntax errors if detected.
Reporter:Steve Murphy (murf)Labels:
Date Opened:2005-06-18 02:31:20Date Closed:2008-01-15 15:38:54.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) check_expr_update.patch
( 1) check_expr_update.patch3
( 2) check_expr_update.patch3a
( 3) diff.check_expr.upgrade2
Description:I've thought about it, and thought it might be handy if check_expr
not only checked for possible probs with 2.5.31 syntax differences, but
also thought it might be more generally useful if it did a
brain-dead variable reference substitution, and then ran the
parser on the result, generating syntax errors if they occur.
That way, you don't have to use asterisk to debug your expressions.

I attach a patch to add a little to the Makefile, to make check_expr
link against the ast_expr.a file, and the mods to check_expr.c, and some
new verbage to explain it all in README.variables.
Comments:By: Kevin P. Fleming (kpfleming) 2005-06-20 18:51:34

This looks like a good addition, but the code formatting does not follow the Asterisk coding guidelines. Please post a corrected patch :-)
By: Steve Murphy (murf) 2005-06-21 00:00:59

OK. There it be.

I THINK I've followed all the guidelines.

Let me know where I missed. I even threw in some code to print line numbers
with the evaluations, so they can be traced more easily to the
source lines.
By: Steve Murphy (murf) 2005-06-21 14:12:11

To my horror, I realized that the patch to utils/Makefile was left out of the new patch. I have uploaded another patch that includes the minor mod to the makefile.
By: Kevin P. Fleming (kpfleming) 2005-06-21 15:05:59

I get a 'malformed patch' error when trying to apply this patch to CVS HEAD.

The remaining formatting issues are extra whitespace or lack of whitespace. For example:

if( ((*cp == '>'||*cp == '<' ||*cp=='!') && *(cp+1) == '=' ) ) {

should be:

if ((*cp == '>' || *cp == '<' || *cp == '!') && (*(cp+1) == '=')) {

In other words, if/while/do blocks should not look like function calls, there is no need for spacing after '(' or before ')'.
By: Steve Murphy (murf) 2005-06-21 16:08:56

Sorry about the patch. Never try to have multiple bugs open with overlapping patches. I think I have that licked. The #3 patch has been uploaded. The if/while/for spacing has been corrected. I threw in a fix to the README.variables that corrects the use of 'operator' to 'application', in reference to GotoIf.
Hope this doesn't get anybody's mind out of joint, but it IS a valid correction.

Everything else is the same, only better now. ;^)
By: Kevin P. Fleming (kpfleming) 2005-06-21 16:16:20

Looks good, committed to CVS HEAD, thanks!
By: Steve Murphy (murf) 2005-06-21 16:20:41

Oh, I am truly sorry! I mucked up. That patch had some extras in the
README.variables, that were meant for the other (conditional expr) upgrade.

I forgot to save the edited patch. I'm going to upload the "correct" patch
now. Back out README.variables, and apply just this (soon to be submitted)
patch for just that file.

Again, sorry! Somedays I'm good, and somedays I'm not.
By: Kevin P. Fleming (kpfleming) 2005-06-21 18:09:50

I'm going to commit the conditional operator stuff anyway, so why don't you just update the patch in that bug based on what README.variables looks like in CVS now, rather than backing out any changes already committed.
By: Digium Subversion (svnbot) 2008-01-15 15:38:54.000-0600

Repository: asterisk
Revision: 5959

U   trunk/utils/Makefile
U   trunk/utils/check_expr.c

------------------------------------------------------------------------
r5959 | kpfleming | 2008-01-15 15:38:53 -0600 (Tue, 15 Jan 2008) | 2 lines

enhance check_expr to check for parsing errors in dialplans (bug ASTERISK-4441)

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=5959