Re: [PATCH 1/5] Refactor the command parsing framework
Hi Zhenhua, --- gatchat/gatserver.c | 182 +++--- gatchat/gatserver.h | 7 ++- 2 files changed, 133 insertions(+), 56 deletions(-) - at_command_notify(server, buf, prefix, type); + notify = at_node_new(server, buf, prefix, type); + if (!notify) + goto error; /* Also consume the terminating null */ - return i + 1; + server-read_pos += i + 1; + server-notify_source = g_idle_add_full(G_PRIORITY_DEFAULT, + at_command_notify, + notify, g_free); + + return; + +error: + g_free(notify); + + g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR); } Err, OK stop right there. This is really way too complicated. How about we simply set a flag before calling at_command_notify. If after executing it send_final or send_ext_final response has been sent, then we continue processing, otherwise we restart when send_ext_final or send_final will be called again. You really don't need to touch this function at all. +typedef void (*GAtServerNotifyCallback)(gpointer user_data); + Get rid of this, not necessary. typedef void (*GAtServerNotifyFunc)(GAtServerRequestType type, - GAtResult *result, gpointer user_data); + GAtResult *result, + gpointer user_data, + GAtServerNotifyCallback cb, + gpointer cb_data); Keep the NotifyFunc the way it is, your changes are really not required. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
RE: [PATCH 1/5] Refactor the command parsing framework
Hi Denis, Denis Kenzior wrote: Hi Zhenhua, --- gatchat/gatserver.c | 182 +++--- gatchat/gatserver.h | 7 ++- 2 files changed, 133 insertions(+), 56 deletions(-) -at_command_notify(server, buf, prefix, type); +notify = at_node_new(server, buf, prefix, type); +if (!notify) +goto error; /* Also consume the terminating null */ -return i + 1; +server-read_pos += i + 1; +server-notify_source = g_idle_add_full(G_PRIORITY_DEFAULT, +at_command_notify, + notify, g_free); + +return; + +error: +g_free(notify); + +g_at_server_send_final(server, G_AT_SERVER_RESULT_ERROR); } Err, OK stop right there. This is really way too complicated. How about we simply set a flag before calling at_command_notify. If after executing it send_final or send_ext_final response has been sent, then we continue processing, otherwise we restart when send_ext_final or send_final will be called again. You really don't need to touch this function at all. OK. So the problem is if the at_command_notify is blocked by user callback, the mainloop won't have chance to read data in from non-blocking I/O I guess. That's the reason I added this. If we don't need to handle such case, then the logic could be much simplier. We could discuss it on IRC. +typedef void (*GAtServerNotifyCallback)(gpointer user_data); + Get rid of this, not necessary. typedef void (*GAtServerNotifyFunc)(GAtServerRequestType type, -GAtResult *result, gpointer user_data); +GAtResult *result, +gpointer user_data, +GAtServerNotifyCallback cb, +gpointer cb_data); Keep the NotifyFunc the way it is, your changes are really not required. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono Regards, Zhenhua ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
Re: [PATCH 1/5] Refactor the command parsing framework
Hi Zhenhua Err, OK stop right there. This is really way too complicated. How about we simply set a flag before calling at_command_notify. If after executing it send_final or send_ext_final response has been sent, then we continue processing, otherwise we restart when send_ext_final or send_final will be called again. You really don't need to touch this function at all. OK. So the problem is if the at_command_notify is blocked by user callback, the mainloop won't have chance to read data in from non-blocking I/O I guess. That's the reason I added this. If we don't need to handle such case, then the logic could be much simplier. We could discuss it on IRC. So what you want to assume here is that for long running operations the callback with schedule its own g_sources and will eventually call send_final. This works because the server can only have one outstanding command at a time. You don't need to worry about blocking semantics here (they just work) and we won't consider re-entrant event loops for now. Regards, -Denis ___ ofono mailing list ofono@ofono.org http://lists.ofono.org/listinfo/ofono
