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 _______________________________________________ ofono mailing list [email protected] http://lists.ofono.org/listinfo/ofono
