Patch on unsupported AT command

[Gu, Yang]

>-----Original Message-----
>From: ofono-bounces at ofono.org [mailto:ofono-bounces at ofono.org] On Behalf Of
>Marcel Holtmann
>Sent: Saturday, November 21, 2009 6:42 PM
>To: ofono at ofono.org
>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
>ofono at ofono.org
>http://lists.ofono.org/listinfo/ofono
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Framework-to-support-non-standard-terminator.patch
Type: application/octet-stream
Size: 3892 bytes
Desc: 0001-Framework-to-support-non-standard-terminator.patch
URL: <http://lists.ofono.org/pipermail/ofono/attachments/20091123/33be70cb/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Support-Huawei-specific-terminator.patch
Type: application/octet-stream
Size: 713 bytes
Desc: 0002-Support-Huawei-specific-terminator.patch
URL: <http://lists.ofono.org/pipermail/ofono/attachments/20091123/33be70cb/attachment-0001.obj>