>-----Original Message----- >From: [email protected] [mailto:[email protected]] On Behalf Of >Marcel Holtmann >Sent: Saturday, November 21, 2009 6:42 PM >To: [email protected] >Subject: RE: Patch on unsupported AT command > >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.
Code is modified according to your comment. Please have a review! >Regards > >Marcel > > >_______________________________________________ >ofono mailing list >[email protected] >http://lists.ofono.org/listinfo/ofono
0001-Framework-to-support-non-standard-terminator.patch
Description: 0001-Framework-to-support-non-standard-terminator.patch
0002-Support-Huawei-specific-terminator.patch
Description: 0002-Support-Huawei-specific-terminator.patch
_______________________________________________ ofono mailing list [email protected] http://lists.ofono.org/listinfo/ofono
