Re: [PATCH 03/11] Add basic command parsing

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1847 bytes --]

Hi Zhenhua,

> According to V.250 5.3.1, the basic command is either a single
> character or the '&' followed by a single character.
> ---
>  gatchat/gatserver.c |   68
>  ++++++++++++++++++++++++++++++++++++++++++++++++++- 1 files changed, 
67
>  insertions(+), 1 deletions(-)
> 
> diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
> index fb82ad4..b68894d 100644
> --- a/gatchat/gatserver.c
> +++ b/gatchat/gatserver.c
> @@ -211,10 +211,67 @@ static char *parse_extended_command(GAtServer
>  *server, const char *buf, return NULL;
>  }
> 
> +static gboolean get_basic_prefix(const char *buf, char *prefix)
> +{
> +	char c = *buf;
> +
> +	if (g_ascii_isalpha(c)) {

if (!g_ascii_isalpha && c != '&')
	return

No nested ifs if you can avoid them please.

> +		c = g_ascii_toupper(c);
> +		if (c == 'S') {
> +			int i = 0;
> +
> +			prefix[0] = 'S';
> +
> +			/* V.250 5.3.2 'S' command follows with
> +			 * a parameter number.
> +			 */
> +			while (g_ascii_isdigit(buf[++i]))
> +				prefix[i] = buf[i];
> +
> +			prefix[i] = '\0';
> +		} else {
> +			prefix[0] = c;
> +			prefix[1] = '\0';
> +		}
> +	} else if (c == '&') {
> +		prefix[0] = '&';
> +		prefix[1] = g_ascii_toupper(buf[1]);
> +		prefix[2] = '\0';
> +	} else
> +		return FALSE;
> +
> +	return TRUE;
> +}
> +
>  static char *parse_next_command(GAtServer *server, const char *buf,
> @@ -263,6 +320,15 @@ static void server_parse_line(GAtServer *server, char
>  *line) break;
>  		}
> 
> +		/* Commands like ATA, ATD, ATZ cause the remainder line
> +		 * to be ignored.
> +		 */
> +		if (!strcmp(prefix, "A") || !strcmp(prefix, "D") ||
> +				!strcmp(prefix, "Z")) {
> +			g_free(command);
> +			break;
> +		}
> +

strcmp might be overkill here

>  		buf += strlen(command);
> 
>  		g_free(command);
>