Hi Yang,

> Thanks for the comments. I have split the patch to two separate ones.
>

Two problems:

>+void g_at_chat_add_terminator(GAtChat *chat, const char *terminator,
>+                                      int len, gboolean success)
>+{
>+      struct terminator_info *ti = g_new0(struct terminator_info, 1);
>+      ti->terminator = terminator;
>+      ti->len = len;
>+      ti->success = success;
>+      chat->terminator_table = g_slist_prepend(chat->terminator_table, ti);
>+}

You're not strdup'ing the terminator string here.  For safety reasons I 
suggest this be done.

>+      g_at_chat_add_terminator(chat, "+EXT ERROR:", 11, FALSE);
>+      g_at_chat_add_terminator(chat, "+CME ERROR:", 11, FALSE);
>+      g_at_chat_add_terminator(chat, "+CMS ERROR:", 11, FALSE);
>+      g_at_chat_add_terminator(chat, "NO ANSWER", -1, FALSE);
>+      g_at_chat_add_terminator(chat, "CONNECT", -1, TRUE);
>+      g_at_chat_add_terminator(chat, "NO CARRIER", -1, FALSE);
>+      g_at_chat_add_terminator(chat, "BUSY", -1, FALSE);
>+      g_at_chat_add_terminator(chat, "NO DIALTONE", -1, FALSE);
>+      g_at_chat_add_terminator(chat, "ERROR", -1, FALSE);
>+      g_at_chat_add_terminator(chat, "OK", -1, TRUE);

I really don't like this.  Lets keep the non-standard terminators in a 
separate list.  I don't want the vast majority of the drivers incurring the 
cost of multiple g_new/g_frees.

Regards,
-Denis
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono

Reply via email to