RE: Patch on unsupported AT command

[Marcel Holtmann <marcel@holtmann.org>]

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

Hi Yang,

> >> >> >+	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.
> >> >
> >> >I have to agree on this. We should keep the penalty for well behaving
> >> >cards as small as possible.
> >>
> >> Thank you for the comments. Modified patches are attached!
> >
> >please do casts with a space between. Like (char *) terminator etc. Also
> >why do you bother with making it const. Just leave that out. Since you
> >do actually copy the string anyway.
> 
> Fixed!

I don't like this casting at all actually. Please just store the
terminator as char and not const char. That is internal code anyway and
you do allocate it. So no need to mark it as read-only. Also please use
g_strdup since you are using g_free to free it.

+static gboolean meet_terminator(GAtChat *chat,
+                               struct terminator_info *info, char *line)

About this function name, I don't really like it since it confuses the
hell out of me what it is meant do be doing. I do get what you are
trying to do here, but I would just name it check_terminator and then
leave the g_at_chat_finish_command() call in the original code.

Regards

Marcel