[PATCH 0/3] Updated patches to handle the asynchronized callback

[PATCH 0/3] Updated patches to handle the asynchronized callback

[Zhenhua Zhang]

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

Hi,

I updated the patches according to the comments and did following changes:

1. removed last_result since it seems no use at all.
2. rename g_at_server_send_flush to g_at_server_send_result. It makes code easier to read.
3. add parse_ready flag to parse one command line at one time.

Please review them. Thanks!

[PATCH 1/3] Refactor the command parsing

[Zhenhua Zhang]

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

---
 gatchat/gatserver.c |  103 +++++++++++++++++++++++++++-----------------------
 1 files changed, 56 insertions(+), 47 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index c75fbf5..07999a8 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -112,6 +112,8 @@ struct _GAtServer {
 	guint max_read_attempts;		/* Max reads per select */
 	enum ParserState parser_state;
 	gboolean destroyed;			/* Re-entrancy guard */
+	char *read_line;			/* Current read line */
+	unsigned int read_pos;			/* Current read offset */
 };
 
 static void g_at_server_wakeup_writer(GAtServer *server);
@@ -215,7 +217,7 @@ static void at_command_notify(GAtServer *server, char *command,
 	g_slist_free(result.lines);
 }
 
-static unsigned int parse_extended_command(GAtServer *server, char *buf)
+static void parse_extended_command(GAtServer *server, char *buf)
 {
 	const char *valid_extended_chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
 						"0123456789!%-./:_";
@@ -230,7 +232,7 @@ static unsigned int parse_extended_command(GAtServer *server, char *buf)
 	prefix_len = strcspn(buf, separators);
 
 	if (prefix_len > 17 || prefix_len < 2)
-		return 0;
+		goto error;
 
 	/* Convert to upper case, we will always use upper case naming */
 	for (i = 0; i < prefix_len; i++)
@@ -239,17 +241,17 @@ static unsigned int parse_extended_command(GAtServer *server, char *buf)
 	prefix[prefix_len] = '\0';
 
 	if (strspn(prefix + 1, valid_extended_chars) != (prefix_len - 1))
-		return 0;
+		goto error;
 
 	/*
 	 * V.250 Section 5.4.1: "The first character following "+" shall be
 	 * an alphabetic character in the range "A" through "Z".
 	 */
 	if (prefix[1] <= 'A' || prefix[1] >= 'Z')
-		return 0;
+		goto error;
 
 	if (buf[i] != '\0' && buf[i] != ';' && buf[i] != '?' && buf[i] != '=')
-		return 0;
+		goto error;
 
 	type = G_AT_SERVER_REQUEST_TYPE_COMMAND_ONLY;
 
@@ -265,16 +267,16 @@ static unsigned int parse_extended_command(GAtServer *server, char *buf)
 
 		if (buf[i] == '?') {
 			if (seen_question || seen_equals)
-				return 0;
+				goto error;
 
 			if (buf[i + 1] != '\0' && buf[i + 1] != ';')
-				return 0;
+				goto error;
 
 			seen_question = TRUE;
 			type = G_AT_SERVER_REQUEST_TYPE_QUERY;
 		} else if (buf[i] == '=') {
 			if (seen_equals || seen_question)
-				return 0;
+				goto error;
 
 			seen_equals = TRUE;
 
@@ -291,10 +293,15 @@ next:
 	/* We can scratch in this buffer, so mark ';' as null */
 	buf[i] = '\0';
 
+	/* Also consume the terminating null */
+	server->read_pos += i + 1;
+
 	at_command_notify(server, buf, prefix, type);
 
-	/* Also consume the terminating null */
-	return i + 1;
+	return;
+
+error:
+	g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
 }
 
 static int get_basic_prefix_size(const char *buf)
@@ -336,16 +343,20 @@ static int get_basic_prefix_size(const char *buf)
 	return 0;
 }
 
-static unsigned int parse_basic_command(GAtServer *server, char *buf)
+static void parse_basic_command(GAtServer *server, char *buf)
 {
 	gboolean seen_equals = FALSE;
 	char prefix[4], tmp;
-	unsigned int i, prefix_size;
+	unsigned int i, prefix_size, end;
 	GAtServerRequestType type;
 
 	prefix_size = get_basic_prefix_size(buf);
 	if (prefix_size == 0)
-		return 0;
+		goto error;
+
+	/* Handle S-parameter with 100+ */
+	if (prefix_size > 3)
+		goto error;
 
 	i = prefix_size;
 	prefix[0] = g_ascii_toupper(buf[0]);
@@ -390,33 +401,39 @@ static unsigned int parse_basic_command(GAtServer *server, char *buf)
 	}
 
 done:
-	if (prefix_size <= 3) {
-		memcpy(prefix + 1, buf + 1, prefix_size - 1);
-		prefix[prefix_size] = '\0';
-
-		tmp = buf[i];
-		buf[i] = '\0';
-		at_command_notify(server, buf, prefix, type);
-		buf[i] = tmp;
-	} else /* Handle S-parameter with 100+ */
-		g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
+	end = i;
 
 	/* Commands like ATA, ATZ cause the remainder line
 	 * to be ignored.
 	 */
 	if (prefix[0] == 'A' || prefix[0] == 'Z')
-		return strlen(buf);
+		i = strlen(buf);
 
 	/* Consume the seperator ';' */
 	if (buf[i] == ';')
 		i += 1;
 
-	return i;
+	/* Update read offset before notify the callback */
+	server->read_pos += i;
+
+	memcpy(prefix + 1, buf + 1, prefix_size - 1);
+	prefix[prefix_size] = '\0';
+
+	tmp = buf[end];
+	buf[end] = '\0';
+	at_command_notify(server, buf, prefix, type);
+	buf[end] = tmp;
+
+	return;
+
+error:
+	g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
 }
 
-static void server_parse_line(GAtServer *server, char *line)
+static void server_parse_line(GAtServer *server)
 {
-	unsigned int pos = 0;
+	char *line = server->read_line;
+	unsigned int pos = server->read_pos;
 	unsigned int len = strlen(line);
 
 	if (len == 0) {
@@ -424,22 +441,10 @@ static void server_parse_line(GAtServer *server, char *line)
 		return;
 	}
 
-	while (pos < len) {
-		unsigned int consumed;
-
-		if (is_extended_command_prefix(line[pos]))
-			consumed = parse_extended_command(server, line + pos);
-		else
-			consumed = parse_basic_command(server, line + pos);
-
-		if (consumed == 0) {
-			g_at_server_send_final(server,
-						G_AT_SERVER_RESULT_ERROR);
-			break;
-		}
-
-		pos += consumed;
-	}
+	if (is_extended_command_prefix(line[pos]))
+		parse_extended_command(server, line + pos);
+	else
+		parse_basic_command(server, line + pos);
 }
 
 static enum ParserResult server_feed(GAtServer *server,
@@ -621,11 +626,13 @@ static void new_bytes(GAtServer *p)
 
 		case PARSER_RESULT_COMMAND:
 		{
-			char *line = extract_line(p);
+			g_free(p->read_line);
 
-			if (line) {
-				server_parse_line(p, line);
-				g_free(line);
+			p->read_line = extract_line(p);
+			if (p->read_line) {
+				p->read_pos = 0;
+
+				server_parse_line(p);
 			} else
 				g_at_server_send_final(p,
 						G_AT_SERVER_RESULT_ERROR);
@@ -795,6 +802,8 @@ static void g_at_server_cleanup(GAtServer *server)
 	g_hash_table_destroy(server->command_list);
 	server->command_list = NULL;
 
+	g_free(server->read_line);
+
 	server->channel = NULL;
 }
 
-- 
1.6.6.1

[PATCH 2/3] Add server send result code

[Zhenhua Zhang]

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

---
 gatchat/gatserver.c |   98 ++++++++++++++++++++++++++++++++++++++++++++-------
 gatchat/gatserver.h |   17 +++++++++
 2 files changed, 102 insertions(+), 13 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index 07999a8..cad9d91 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -33,6 +33,8 @@
 #include "gatserver.h"
 
 #define BUF_SIZE 4096
+/* <cr><lf> + the max length of information text + <cr><lf> */
+#define MAX_TEXT_SIZE 2052
 /* #define WRITE_SCHEDULER_DEBUG 1 */
 
 enum ParserState {
@@ -117,6 +119,7 @@ struct _GAtServer {
 };
 
 static void g_at_server_wakeup_writer(GAtServer *server);
+static void server_parse_line(GAtServer *server);
 
 static struct ring_buffer *allocate_next(GAtServer *server)
 {
@@ -158,11 +161,11 @@ static void send_common(GAtServer *server, const char *buf, unsigned int len)
 	g_at_server_wakeup_writer(server);
 }
 
-static void g_at_server_send_final(GAtServer *server, GAtServerResult result)
+static void send_result_common(GAtServer *server, const char *result)
+
 {
 	struct v250_settings v250 = server->v250;
-	const char *result_str = server_result_to_string(result);
-	char buf[1024];
+	char buf[MAX_TEXT_SIZE];
 	char t = v250.s3;
 	char r = v250.s4;
 	unsigned int len;
@@ -170,19 +173,88 @@ static void g_at_server_send_final(GAtServer *server, GAtServerResult result)
 	if (v250.quiet)
 		return;
 
-	if (result_str == NULL)
+	if (result == NULL)
 		return;
 
 	if (v250.is_v1)
-		len = snprintf(buf, sizeof(buf), "%c%c%s%c%c", t, r, result_str,
+		len = snprintf(buf, sizeof(buf), "%c%c%s%c%c", t, r, result,
 				t, r);
 	else
-		len = snprintf(buf, sizeof(buf), "%u%c", (unsigned int) result,
+		len = snprintf(buf, sizeof(buf), "%s%c", result,
 				t);
 
 	send_common(server, buf, MIN(len, sizeof(buf)-1));
 }
 
+static void g_at_server_send_result(GAtServer *server, GAtServerResult result)
+{
+	char buf[1024];
+
+	if (server->v250.is_v1)
+		sprintf(buf, "%s", server_result_to_string(result));
+	else
+		sprintf(buf, "%u", (unsigned int)result);
+
+	send_result_common(server, buf);
+}
+
+void g_at_server_send_final(GAtServer *server, GAtServerResult result)
+{
+	char *line = server->read_line;
+	unsigned int pos = server->read_pos;
+	unsigned int len = strlen(line);
+
+	/* Continue only if the result is OK and we have further commands */
+	if (result == G_AT_SERVER_RESULT_OK && pos < len)
+		server_parse_line(server);
+	else if (result == G_AT_SERVER_RESULT_EXT_ERROR)
+		;	/* Skip */
+	else
+		g_at_server_send_result(server, result);
+}
+
+void g_at_server_send_ext_final(GAtServer *server, const char *result)
+{
+	send_result_common(server, result);
+}
+
+void g_at_server_send_intermediate(GAtServer *server, const char *result)
+{
+	send_result_common(server, result);
+}
+
+void g_at_server_send_unsolicited(GAtServer *server, const char *result)
+{
+	send_result_common(server, result);
+}
+
+void g_at_server_send_info_text(GAtServer *server, GSList *text)
+{
+	char buf[MAX_TEXT_SIZE];
+	char t = server->v250.s3;
+	char r = server->v250.s4;
+	unsigned int len;
+	GSList *l = text;
+	char *line;
+
+	if (!text)
+		return;
+
+	while (l) {
+		line = l->data;
+		if (!line)
+			return;
+
+		len = snprintf(buf, sizeof(buf), "%c%c%s", t, r, line);
+		send_common(server, buf, MIN(len, sizeof(buf)-1));
+
+		l = l->next;
+	}
+
+	len = snprintf(buf, sizeof(buf), "%c%c", t, r);
+	send_common(server, buf, len);
+}
+
 static inline gboolean is_extended_command_prefix(const char c)
 {
 	switch (c) {
@@ -205,7 +277,7 @@ static void at_command_notify(GAtServer *server, char *command,
 	node = g_hash_table_lookup(server->command_list, prefix);
 
 	if (node == NULL) {
-		g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
+		g_at_server_send_result(server, G_AT_SERVER_RESULT_ERROR);
 		return;
 	}
 
@@ -301,7 +373,7 @@ next:
 	return;
 
 error:
-	g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
+	g_at_server_send_result(server, G_AT_SERVER_RESULT_ERROR);
 }
 
 static int get_basic_prefix_size(const char *buf)
@@ -427,7 +499,7 @@ done:
 	return;
 
 error:
-	g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
+	g_at_server_send_result(server, G_AT_SERVER_RESULT_ERROR);
 }
 
 static void server_parse_line(GAtServer *server)
@@ -437,7 +509,7 @@ static void server_parse_line(GAtServer *server)
 	unsigned int len = strlen(line);
 
 	if (len == 0) {
-		g_at_server_send_final(server, G_AT_SERVER_RESULT_OK);
+		g_at_server_send_result(server, G_AT_SERVER_RESULT_OK);
 		return;
 	}
 
@@ -620,7 +692,7 @@ static void new_bytes(GAtServer *p)
 			 * According to section 5.2.4 and 5.6 of V250,
 			 * Empty commands must be OK by the DCE
 			 */
-			g_at_server_send_final(p, G_AT_SERVER_RESULT_OK);
+			g_at_server_send_result(p, G_AT_SERVER_RESULT_OK);
 			ring_buffer_drain(p->read_buf, p->read_so_far);
 			break;
 
@@ -634,14 +706,14 @@ static void new_bytes(GAtServer *p)
 
 				server_parse_line(p);
 			} else
-				g_at_server_send_final(p,
+				g_at_server_send_result(p,
 						G_AT_SERVER_RESULT_ERROR);
 			break;
 		}
 
 		case PARSER_RESULT_REPEAT_LAST:
 			/* TODO */
-			g_at_server_send_final(p, G_AT_SERVER_RESULT_OK);
+			g_at_server_send_result(p, G_AT_SERVER_RESULT_OK);
 			ring_buffer_drain(p->read_buf, p->read_so_far);
 			break;
 
diff --git a/gatchat/gatserver.h b/gatchat/gatserver.h
index 2ae19ca..a508be6 100644
--- a/gatchat/gatserver.h
+++ b/gatchat/gatserver.h
@@ -87,6 +87,23 @@ gboolean g_at_server_register(GAtServer *server, char *prefix,
 					GDestroyNotify destroy_notify);
 gboolean g_at_server_unregister(GAtServer *server, const char *prefix);
 
+/* Send a final result code. E.g. G_AT_SERVER_RESULT_NO_DIALTONE */
+void g_at_server_send_final(GAtServer *server, GAtServerResult result);
+
+/* Send an extended final result code. E.g. +CME ERROR: SIM failure. */
+void g_at_server_send_ext_final(GAtServer *server, const char *result);
+
+/* Send an intermediate result code to report the progress. E.g. CONNECT */
+void g_at_server_send_intermediate(GAtServer *server, const char *result);
+
+/* Send an unsolicited result code. E.g. RING */
+void g_at_server_send_unsolicited(GAtServer *server, const char *result);
+
+/* Send an information text. The text could contain multiple lines. Each
+ * line, including line terminators, should not exceed 2048 characters.
+ */
+void g_at_server_send_info_text(GAtServer *server, GSList *text);
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.6.6.1

[PATCH 3/3] Add flag to parse one command line at once

[Zhenhua Zhang]

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

---
 gatchat/gatserver.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
index cad9d91..251c3b8 100644
--- a/gatchat/gatserver.c
+++ b/gatchat/gatserver.c
@@ -116,6 +116,7 @@ struct _GAtServer {
 	gboolean destroyed;			/* Re-entrancy guard */
 	char *read_line;			/* Current read line */
 	unsigned int read_pos;			/* Current read offset */
+	gboolean parse_ready;			/* Ready to parse command */
 };
 
 static void g_at_server_wakeup_writer(GAtServer *server);
@@ -196,6 +197,8 @@ static void g_at_server_send_result(GAtServer *server, GAtServerResult result)
 		sprintf(buf, "%u", (unsigned int)result);
 
 	send_result_common(server, buf);
+
+	server->parse_ready = TRUE;
 }
 
 void g_at_server_send_final(GAtServer *server, GAtServerResult result)
@@ -216,6 +219,8 @@ void g_at_server_send_final(GAtServer *server, GAtServerResult result)
 void g_at_server_send_ext_final(GAtServer *server, const char *result)
 {
 	send_result_common(server, result);
+
+	server->parse_ready = TRUE;
 }
 
 void g_at_server_send_intermediate(GAtServer *server, const char *result)
@@ -671,7 +676,7 @@ static void new_bytes(GAtServer *p)
 	unsigned char *buf = ring_buffer_read_ptr(p->read_buf, p->read_so_far);
 	enum ParserResult result;
 
-	while (p->channel && (p->read_so_far < len)) {
+	while (p->channel && p->parse_ready && (p->read_so_far < len)) {
 		gsize rbytes = MIN(len - p->read_so_far, wrap - p->read_so_far);
 		result = server_feed(p, (char *)buf, &rbytes);
 
@@ -703,6 +708,7 @@ static void new_bytes(GAtServer *p)
 			p->read_line = extract_line(p);
 			if (p->read_line) {
 				p->read_pos = 0;
+				p->parse_ready = FALSE;
 
 				server_parse_line(p);
 			} else
@@ -945,6 +951,7 @@ GAtServer *g_at_server_new(GIOChannel *io)
 	server->ref_count = 1;
 	v250_settings_create(&server->v250);
 	server->channel = io;
+	server->parse_ready = TRUE;
 	server->command_list = g_hash_table_new_full(g_str_hash, g_str_equal,
 							g_free,
 							at_notify_node_destroy);
-- 
1.6.6.1

Re: [PATCH 1/3] Refactor the command parsing

[Denis Kenzior]

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

Hi Zhenhua,

> ---
>  gatchat/gatserver.c |  103
>  +++++++++++++++++++++++++++----------------------- 1 files changed, 56
>  insertions(+), 47 deletions(-)
> 
> diff --git a/gatchat/gatserver.c b/gatchat/gatserver.c
> index c75fbf5..07999a8 100644
> --- a/gatchat/gatserver.c
> +++ b/gatchat/gatserver.c
> @@ -112,6 +112,8 @@ struct _GAtServer {
>  	guint max_read_attempts;		/* Max reads per select */
>  	enum ParserState parser_state;
>  	gboolean destroyed;			/* Re-entrancy guard */
> +	char *read_line;			/* Current read line */

How about char *last_cmd;

> +	unsigned int read_pos;			/* Current read offset */

And cur_pos;

>  };
> 
>  static void g_at_server_wakeup_writer(GAtServer *server);
> @@ -215,7 +217,7 @@ static void at_command_notify(GAtServer *server, char
>  *command, g_slist_free(result.lines);
>  }
> 
> -static unsigned int parse_extended_command(GAtServer *server, char *buf)
> +static void parse_extended_command(GAtServer *server, char *buf)
>  {
>  	const char *valid_extended_chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
>  						"0123456789!%-./:_";
> @@ -230,7 +232,7 @@ static unsigned int parse_extended_command(GAtServer
>  *server, char *buf) prefix_len = strcspn(buf, separators);
> 
>  	if (prefix_len > 17 || prefix_len < 2)
> -		return 0;
> +		goto error;
> 
>  	/* Convert to upper case, we will always use upper case naming */
>  	for (i = 0; i < prefix_len; i++)
> @@ -239,17 +241,17 @@ static unsigned int parse_extended_command(GAtServer
>  *server, char *buf) prefix[prefix_len] = '\0';
> 
>  	if (strspn(prefix + 1, valid_extended_chars) != (prefix_len - 1))
> -		return 0;
> +		goto error;
> 
>  	/*
>  	 * V.250 Section 5.4.1: "The first character following "+" shall be
>  	 * an alphabetic character in the range "A" through "Z".
>  	 */
>  	if (prefix[1] <= 'A' || prefix[1] >= 'Z')
> -		return 0;
> +		goto error;
> 
>  	if (buf[i] != '\0' && buf[i] != ';' && buf[i] != '?' && buf[i] != '=')
> -		return 0;
> +		goto error;
> 
>  	type = G_AT_SERVER_REQUEST_TYPE_COMMAND_ONLY;
> 
> @@ -265,16 +267,16 @@ static unsigned int parse_extended_command(GAtServer
>  *server, char *buf)
> 
>  		if (buf[i] == '?') {
>  			if (seen_question || seen_equals)
> -				return 0;
> +				goto error;
> 
>  			if (buf[i + 1] != '\0' && buf[i + 1] != ';')
> -				return 0;
> +				goto error;
> 
>  			seen_question = TRUE;
>  			type = G_AT_SERVER_REQUEST_TYPE_QUERY;
>  		} else if (buf[i] == '=') {
>  			if (seen_equals || seen_question)
> -				return 0;
> +				goto error;
> 
>  			seen_equals = TRUE;
> 
> @@ -291,10 +293,15 @@ next:
>  	/* We can scratch in this buffer, so mark ';' as null */
>  	buf[i] = '\0';
> 
> +	/* Also consume the terminating null */
> +	server->read_pos += i + 1;
> +
>  	at_command_notify(server, buf, prefix, type);
> 
> -	/* Also consume the terminating null */
> -	return i + 1;
> +	return;
> +
> +error:
> +	g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
>  }

So I suggest you leave this function as is and do the read_pos logic in 
server_parse_line.

> 
>  static int get_basic_prefix_size(const char *buf)
> @@ -336,16 +343,20 @@ static int get_basic_prefix_size(const char *buf)
>  	return 0;
>  }
> 
> -static unsigned int parse_basic_command(GAtServer *server, char *buf)
> +static void parse_basic_command(GAtServer *server, char *buf)
>  {
>  	gboolean seen_equals = FALSE;
>  	char prefix[4], tmp;
> -	unsigned int i, prefix_size;
> +	unsigned int i, prefix_size, end;
>  	GAtServerRequestType type;
> 
>  	prefix_size = get_basic_prefix_size(buf);
>  	if (prefix_size == 0)
> -		return 0;
> +		goto error;
> +
> +	/* Handle S-parameter with 100+ */
> +	if (prefix_size > 3)
> +		goto error;
> 
>  	i = prefix_size;
>  	prefix[0] = g_ascii_toupper(buf[0]);
> @@ -390,33 +401,39 @@ static unsigned int parse_basic_command(GAtServer
>  *server, char *buf) }
> 
>  done:
> -	if (prefix_size <= 3) {
> -		memcpy(prefix + 1, buf + 1, prefix_size - 1);
> -		prefix[prefix_size] = '\0';
> -
> -		tmp = buf[i];
> -		buf[i] = '\0';
> -		at_command_notify(server, buf, prefix, type);
> -		buf[i] = tmp;
> -	} else /* Handle S-parameter with 100+ */
> -		g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
> +	end = i;
> 
>  	/* Commands like ATA, ATZ cause the remainder line
>  	 * to be ignored.
>  	 */
>  	if (prefix[0] == 'A' || prefix[0] == 'Z')
> -		return strlen(buf);
> +		i = strlen(buf);
> 
>  	/* Consume the seperator ';' */
>  	if (buf[i] == ';')
>  		i += 1;
> 
> -	return i;
> +	/* Update read offset before notify the callback */
> +	server->read_pos += i;
> +
> +	memcpy(prefix + 1, buf + 1, prefix_size - 1);
> +	prefix[prefix_size] = '\0';
> +
> +	tmp = buf[end];
> +	buf[end] = '\0';
> +	at_command_notify(server, buf, prefix, type);
> +	buf[end] = tmp;
> +
> +	return;
> +
> +error:
> +	g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
>  }
> 

As above.

Regards,
-Denis

Re: [PATCH 2/3] Add server send result code

[Denis Kenzior]

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

Hi Zhenhua,

> +static void g_at_server_send_result(GAtServer *server, GAtServerResult
>  result) +{
> +	char buf[1024];
> +
> +	if (server->v250.is_v1)
> +		sprintf(buf, "%s", server_result_to_string(result));
> +	else
> +		sprintf(buf, "%u", (unsigned int)result);
> +
> +	send_result_common(server, buf);
> +}

Please get rid of this function, it serves no purpose and is utterly 
confusing.  If you would put the contents at the else clause of 
g_at_server_send_final then the same thing will be accomplished.

> +
> +void g_at_server_send_final(GAtServer *server, GAtServerResult result)
> +{
> +	char *line = server->read_line;
> +	unsigned int pos = server->read_pos;
> +	unsigned int len = strlen(line);
> +
> +	/* Continue only if the result is OK and we have further commands */
> +	if (result == G_AT_SERVER_RESULT_OK && pos < len)
> +		server_parse_line(server);
> +	else if (result == G_AT_SERVER_RESULT_EXT_ERROR)
> +		;	/* Skip */

Nobody should ever call this function with EXT_ERROR

> +	else
> +		g_at_server_send_result(server, result);
> +}
> +
> +void g_at_server_send_ext_final(GAtServer *server, const char *result)
> +{
> +	send_result_common(server, result);

Why don't we do the same server_parse_line magic here?

> +}
> +
> +void g_at_server_send_intermediate(GAtServer *server, const char *result)
> +{
> +	send_result_common(server, result);
> +}
> +
> +void g_at_server_send_unsolicited(GAtServer *server, const char *result)
> +{
> +	send_result_common(server, result);
> +}
> +
> +void g_at_server_send_info_text(GAtServer *server, GSList *text)
> +{
> +	char buf[MAX_TEXT_SIZE];
> +	char t = server->v250.s3;
> +	char r = server->v250.s4;
> +	unsigned int len;
> +	GSList *l = text;
> +	char *line;
> +
> +	if (!text)
> +		return;
> +
> +	while (l) {
> +		line = l->data;
> +		if (!line)
> +			return;
> +
> +		len = snprintf(buf, sizeof(buf), "%c%c%s", t, r, line);
> +		send_common(server, buf, MIN(len, sizeof(buf)-1));
> +
> +		l = l->next;
> +	}

Using a for loop is more compact here.

> +
> +	len = snprintf(buf, sizeof(buf), "%c%c", t, r);
> +	send_common(server, buf, len);
> +}
> +
>  static inline gboolean is_extended_command_prefix(const char c)
>  {
>  	switch (c) {
> @@ -205,7 +277,7 @@ static void at_command_notify(GAtServer *server, char
>  *command, node = g_hash_table_lookup(server->command_list, prefix);
> 
>  	if (node == NULL) {
> -		g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
> +		g_at_server_send_result(server, G_AT_SERVER_RESULT_ERROR);

Leave it as is

>  		return;
>  	}
> 
> @@ -301,7 +373,7 @@ next:
>  	return;
> 
>  error:
> -	g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
> +	g_at_server_send_result(server, G_AT_SERVER_RESULT_ERROR);

Leave it as is

>  }
> 
>  static int get_basic_prefix_size(const char *buf)
> @@ -427,7 +499,7 @@ done:
>  	return;
> 
>  error:
> -	g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR);
> +	g_at_server_send_result(server, G_AT_SERVER_RESULT_ERROR);

And again,

>  }
> 
>  static void server_parse_line(GAtServer *server)
> @@ -437,7 +509,7 @@ static void server_parse_line(GAtServer *server)
>  	unsigned int len = strlen(line);
> 
>  	if (len == 0) {
> -		g_at_server_send_final(server, G_AT_SERVER_RESULT_OK);
> +		g_at_server_send_result(server, G_AT_SERVER_RESULT_OK);

And again

>  		return;
>  	}
> 
> @@ -620,7 +692,7 @@ static void new_bytes(GAtServer *p)
>  			 * According to section 5.2.4 and 5.6 of V250,
>  			 * Empty commands must be OK by the DCE
>  			 */
> -			g_at_server_send_final(p, G_AT_SERVER_RESULT_OK);
> +			g_at_server_send_result(p, G_AT_SERVER_RESULT_OK);

And here

>  			ring_buffer_drain(p->read_buf, p->read_so_far);
>  			break;
> 
> @@ -634,14 +706,14 @@ static void new_bytes(GAtServer *p)
> 
>  				server_parse_line(p);
>  			} else
> -				g_at_server_send_final(p,
> +				g_at_server_send_result(p,
>  						G_AT_SERVER_RESULT_ERROR);

And here

>  			break;
>  		}
> 
>  		case PARSER_RESULT_REPEAT_LAST:
>  			/* TODO */
> -			g_at_server_send_final(p, G_AT_SERVER_RESULT_OK);
> +			g_at_server_send_result(p, G_AT_SERVER_RESULT_OK);

And again here

>  			ring_buffer_drain(p->read_buf, p->read_so_far);
>  			break;
> 
> diff --git a/gatchat/gatserver.h b/gatchat/gatserver.h
> index 2ae19ca..a508be6 100644
> --- a/gatchat/gatserver.h
> +++ b/gatchat/gatserver.h
> @@ -87,6 +87,23 @@ gboolean g_at_server_register(GAtServer *server, char
>  *prefix, GDestroyNotify destroy_notify);
>  gboolean g_at_server_unregister(GAtServer *server, const char *prefix);
> 
> +/* Send a final result code. E.g. G_AT_SERVER_RESULT_NO_DIALTONE */
> +void g_at_server_send_final(GAtServer *server, GAtServerResult result);
> +
> +/* Send an extended final result code. E.g. +CME ERROR: SIM failure. */
> +void g_at_server_send_ext_final(GAtServer *server, const char *result);
> +
> +/* Send an intermediate result code to report the progress. E.g. CONNECT
>  */ +void g_at_server_send_intermediate(GAtServer *server, const char
>  *result); +
> +/* Send an unsolicited result code. E.g. RING */
> +void g_at_server_send_unsolicited(GAtServer *server, const char *result);
> +
> +/* Send an information text. The text could contain multiple lines. Each
> + * line, including line terminators, should not exceed 2048 characters.
> + */
> +void g_at_server_send_info_text(GAtServer *server, GSList *text);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> 

Regards,
-Denis