[asterisk-dev] expression checking implementation update

[SF Markus Elfring]

Hello,

I have looked at the function "check_expr" in the source file "check_expr.c". I have found some details that can be improved in its design.

1. The input characters were compared to special values more often than it was really required.
2. The buffer that was used to construct a warning message was too big.

Would anybody like to integrate the appended changes into the version repository?

How do you think about the idea that a FSM approach might be necessary to complete the code?
http://en.wikipedia.org/wiki/Finite_state_machine

Regards,
Markus
-------------- next part --------------
78c78
< int check_expr(char *buffer, char *error_report)
---
> unsigned int check_expr(char* buffer, char* error_report)
80,82c80,82
< 	char *cp;
< 	int oplen = 0;
< 	int warn_found = 0;
---
> 	char* cp;
> 	unsigned int oplen = 0;
> 	unsigned int warn_found = 0;
86,103c86,90
< 	for (cp=buffer;*cp;cp++) {
< 		
< 		if (*cp == '|' 
< 			|| *cp == '&'
< 			|| *cp == '='
< 			|| *cp == '>'
< 			|| *cp == '<'
< 			|| *cp == '+'
< 			|| *cp == '-'
< 			|| *cp == '*'
< 			|| *cp == '/'
< 			|| *cp == '%'
< 			|| *cp == '?'
< 			|| *cp == ':'
< 			/*	|| *cp == '('
< 				|| *cp == ')' These are pretty hard to track, as they are in funcalls, etc. */
< 			|| *cp == '"') {
< 			if (*cp == '"') {
---
> 	for (cp = buffer; *cp; ++cp)
> 	{
> 		switch (*cp)
> 		{
> 			case '"':
105,115c92,98
< 				cp++;
< 				while (*cp && *cp != '"')
< 					cp++;
< 				if (*cp == 0) {
< 					fprintf(stderr,"Trouble? Unterminated double quote found at line %d\n",
< 							global_lineno);
< 				}
< 			}
< 			else {
< 				if ((*cp == '>'||*cp == '<' ||*cp=='!') && (*(cp+1) == '=')) {
< 					oplen = 2;
---
> 				while (*(++cp) && *cp != '"') ;
> 
> 				if (*cp == 0)
> 				{
> 					fprintf(stderr,
> 						"Trouble? Unterminated double quote found at line %d\n",
> 						global_lineno);
117,118c100,115
< 				else {
< 					oplen = 1;
---
> 				break;
> 				
> 			case '>':
> 			case '<':
> 			case '!':
> 				if (   (*(cp + 1) == '=')
> 					&& ( ( (cp > buffer) && (*(cp - 1) != ' ') ) || (*(cp + 2) != ' ') ) )
> 				{
> 					char msg[200];
> 					snprintf(msg,
> 						sizeof(msg),
> 						"WARNING: line %d: '%c%c' operator not separated by spaces. This may lead to confusion. You may wish to use double quotes to quote the grouping it is in. Please check!\n",
> 						global_lineno, *cp, *(cp + 1));
> 					strcat(error_report, msg);
> 					++global_warn_count;
> 					++warn_found;
119a117
> 				break;
121,132c119,138
< 				if ((cp > buffer && *(cp-1) != ' ') || *(cp+oplen) != ' ') {
< 					char tbuf[1000];
< 					if (oplen == 1)
< 						sprintf(tbuf,"WARNING: line %d,  '%c' operator not separated by spaces. This may lead to confusion. You may wish to use double quotes to quote the grouping it is in. Please check!\n",
< 								global_lineno, *cp);
< 					else
< 						sprintf(tbuf,"WARNING: line %d,  '%c%c' operator not separated by spaces. This may lead to confusion. You may wish to use double quotes to quote the grouping it is in. Please check!\n",
< 								global_lineno, *cp, *(cp+1));
< 					strcat(error_report,tbuf);
< 
< 					global_warn_count++;
< 					warn_found++;
---
> 			case '|':
> 			case '&':
> 			case '=':
> 			case '+':
> 			case '-':
> 			case '*':
> 			case '/':
> 			case '%':
> 			case '?':
> 			case ':':
> 				if ( ( (cp > buffer) && (*(cp - 1) != ' ') ) || (*(cp + 1) != ' ') )
> 				{
> 					char msg[200];
> 					snprintf(msg,
> 						sizeof(msg),
> 						"WARNING: line %d: '%c' operator not separated by spaces. This may lead to confusion. You may wish to use double quotes to quote the grouping it is in. Please check!\n",
> 						global_lineno, *cp, *(cp + 1));
> 					strcat(error_report, msg);
> 					++global_warn_count;
> 					++warn_found;
134c140
< 			}
---
> 				break;
136a143
>