Hi Zhenhua,
> +
> +#include
> +
> +#include "ringbuffer.h"
> +#include "gatresult.h"
Is this include really necessary?
> +#include "gatserver.h"
> +
> +#define DBG(fmt, arg...) g_print("%s:%s() " fmt, __FILE__, __FUNCTION__ ,
> ## arg) +
Move this to gat.h
> +struct _GAtServer {
> + gint ref_count; /* Ref count */
> + struct v250_settings *v250; /* V.250 command setting */
This one doesn't need to be a pointer.
> + GIOChannel *server_io; /* Server IO */
> + int server_watch; /* Watch for server IO */
> + char *modem_path; /* Emulated modem path */
Get rid of this, there's no purpose for it.
> +static int at_server_parse(GAtServer *server, char *buf)
> +{
> + int res = G_AT_SERVER_RESULT_ERROR;
> + struct v250_settings *v250 = server->v250;
> + gsize i = 0;
> + char c;
> +
> + /* skip space after "AT" or previous command */
> + i = skip_space(buf, i);
> +
> + c = buf[i];
> + /* skip semicolon */
> + if (c == ';')
> + c = buf[++i];
> +
> + if (g_ascii_isalpha(c) || c == '&')
> + res = parse_v250_settings(server, buf + i);
This part makes no sense
> + else if (c == v250->s3)
> + res = G_AT_SERVER_RESULT_OK;
> +
> + return res;
> +}
> +
> +static void parse_buffer(GAtServer *server, unsigned int len)
> +{
> + int res = G_AT_SERVER_RESULT_ERROR;
> + gsize i = 0;
> + char *buf = NULL;
> +
> + g_at_server_ref(server);
Lets get rid of this part for now. It was a necessary evil inside GAtChat
since the client can close the channel in the command result callback. I
don't think this is relevant for GAtServer. If it ever becomes relevant we
can add this back in.
> +
> + buf = g_try_new0(char, len+1);
You're leaking the buf memory in this function.
> +
> + if (!buf)
> + goto out;
> +
> + if (-1 == ring_buffer_read(server->buf, buf, len))
> + goto out;
> +
> + buf[len] = '\0';
> +
> + DBG("received %s\n", buf);
> +
> + /* skip header space */
> + buf += skip_space(buf, i);
> +
> + /* Make sure the command line prefix is "AT" or "at" */
> + if (g_str_has_prefix(buf, "AT") ||
> + g_str_has_prefix(buf, "at"))
> + res = at_server_parse(server, (char *) buf + 2);
> +
> + g_at_server_send_result_code(server, res);
> +
> +out:
> + /* We're overflowing the buffer, shutdown the socket */
> + if (server->buf && ring_buffer_avail(server->buf) == 0)
> + g_at_server_shutdown(server);
> +
> + g_at_server_unref(server);
Same here, see above.
> +}
> +
> +static gboolean received_data(GIOChannel *chan, GIOCondition cond,
> + gpointer data)
> +{
> + GAtServer *server = data;
> + struct v250_settings *v250 = server->v250;
> + GIOError err;
> + gsize rbytes;
> + gsize total_read = 0;
> + unsigned char *total_buf = ring_buffer_write_ptr(server->buf);
You're totally abusing the ring buffer concept here, the fact that this works
is pure luck.
> + static gsize total_len;
> +
> + if (cond & G_IO_NVAL)
> + return FALSE;
> +
> + if (cond & (G_IO_HUP | G_IO_ERR)) {
> + g_at_server_shutdown(server);
> +
> + return FALSE;
> + }
> +
> + /* Regardless of condition, try to read all the data available */
> + do {
> + unsigned char *buf;
> + gsize toread;
> +
> + rbytes = 0;
> +
> + toread = ring_buffer_avail_no_wrap(server->buf);
> +
> + if (toread == 0)
> + break;
> +
> + buf = ring_buffer_write_ptr(server->buf);
> +
> + err = g_io_channel_read(chan, (char *) buf, toread, &rbytes);
> +
> + total_read += rbytes;
> +
> + if (rbytes > 0)
> + ring_buffer_write_advance(server->buf, rbytes);
> +
> + } while (err == G_IO_ERROR_NONE && rbytes > 0);
> +
> + g_at_util_debug_chat(server->debugf, TRUE, (char *) total_buf,
> + total_read, server->debug_data);
> +
> + if (total_read == 0) {
> + g_at_server_shutdown(server);
You should be checking rbytes == 0 && err != EAGAIN here. Your check below is
also redundant.
> +
> + return FALSE;
> + }
> +
> + total_len += total_read;
> +
> + /* Add '\0' to perform strchr */
> + total_buf[total_read] = '\0';
> +
> + /* Parse buffer until we meet the terminator so that
> + * all preceding characters will be processed
> + */
> + if (strchr((char *) total_buf, v250->s3)) {
> + parse_buffer(server, total_len);
> +
> + total_len = 0;
> + }
Should this be inside a while loop? Several commands can come in at once.
> +
> + if (err != G_IO_ERROR_NONE && err != G_I
