Patch on unsupported AT command
Marcel Holtmann
marcel at holtmann.org
Sat Nov 21 02:41:33 PST 2009
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
More information about the ofono
mailing list