gatio: do not read if no read_handler
PATCH 1/1 gatio: do not read if no read_handler
If no read_handler specified, leave data alone in case someone else wants to read it.
--- gatchat/gatio.c | 4 ++++ gatchat/gsmdial.c | 12 ++++-------- 2 files changed, 8 insertions(+), 8 deletions(-)
gatchat/gatio.c +4 -0
@@ -91,6 +91,10 @@ static gboolean received_data(GIOChannel *channel, GIOCondition cond,if (cond &G_IO_NVAL )return FALSE; +/* if nobody wants this data, leave it alone */ +if (io->read_handler ==NULL ) +return TRUE; +/* Regardless of condition, try to read all the data available */ do { toread = ring_buffer_avail_no_wrap(io->buf);
gatchat/gsmdial.c +4 -8
@@ -235,6 +235,8 @@ static void ppp_connect(const char *iface, const char *ip,static void ppp_disconnect (GAtPPPDisconnectReason reason,gpointer user_data) {g_print ("PPP Link down: %d\n" , reason); +g_at_chat_resume (control); +g_at_chat_resume (modem); }static void connect_cb (gboolean ok,GAtResult *result,gpointer user_data)
@@ -249,14 +251,8 @@ static void connect_cb(gboolean ok, GAtResult *result, gpointer user_data)/* get the data IO channel */ channel =g_at_chat_get_channel (modem); -/* -* shutdown gatchat or else it tries to take all the input -* from the modem and does not let PPP get it. -*/ -g_at_chat_unref (control); - control =NULL ; -g_at_chat_unref (modem); - modem =NULL ; +g_at_chat_suspend (control); +g_at_chat_suspend (modem);/* open ppp */ ppp =g_at_ppp_new (channel);
Signature · 1 line
1.6.6.1
Hi Kristen,
Quoted 12 lines
If no
read_handlerspecified, leave data alone in case someone else wants to read it.--- gatchat/gatio.c | 4 ++++ gatchat/gsmdial.c | 12 ++++-------- 2 files changed, 8 insertions(+), 8 deletions(-)gatchat/gatio.c +0
-0
gatchat/gatio.c static gboolean received_data(GIOChannel *channel, · line 97
Quoted 7 lines
gatchat/gatio.c +4
-0@@ -91,6 +91,10 @@ static gboolean received_data(GIOChannel *channel, GIOCondition cond, if (cond & G_IO_NVAL) return FALSE; +/* if nobody wants this data, leave it alone */ +if (io->read_handler ==NULL ) +return TRUE; +
This is a really bad idea, we should never have cases where the read handler is unset, and if we do we should end up closing the socket when the buffer is full.
Quoted 21 lines
gatchat/gatio.c +0
-0/* Regardless of condition, try to read all the data available */ do { toread = ring_buffer_avail_no_wrap(io->buf);gatchat/gsmdial.c +2
-1@@ -235,6 +235,8 @@ static void ppp_connect(const char *iface, const char *ip, static void ppp_disconnect(GAtPPPDisconnectReason reason, gpointer user_data) {g_print ("PPP Link down: %d\n" , reason); +g_at_chat_resume (control); +g_at_chat_resume (modem); }static void connect_cb (gboolean ok,GAtResult *result,gpointer user_data)@@ -249,14 +251,8 @@ static void connect_cb(gboolean ok, GAtResult *result, gpointer user_data) /* get the data IO channel */ channel =g_at_chat_get_channel (modem); -/*
gatchat/gsmdial.c static void connect_cb(gboolean ok, GAtResult *result, · line 256
Quoted 10 lines
gatchat/gsmdial.c +2
-7-* shutdown gatchat or else it tries to take all the input -* from the modem and does not let PPP get it. -*/ -g_at_chat_unref (control); - control =NULL ; -g_at_chat_unref (modem); - modem =NULL ; +g_at_chat_suspend (control); +g_at_chat_suspend (modem);
Please send gsmdial patches separately. Also, can you make g_at_ppp use the new_from_io constructor? In that case you don't need the gatio modifications above.
gatchat/gsmdial.c static void connect_cb(gboolean ok, GAtResult *result, gpointer user_data) · line 258
Quoted 3 lines
gatchat/gsmdial.c +0
-0/* open ppp */ ppp =g_at_ppp_new (channel);
Regards,
-Denis
Quoted 17 lines · On Mon, 10 May 2010 09:11:52 -0500 Denis Kenzior <[email protected]> wrote:
Hi Kristen,
Quoted 15 lines
If no
read_handlerspecified, leave data alone in case someone else wants to read it.--- gatchat/gatio.c | 4 ++++ gatchat/gsmdial.c | 12 ++++-------- 2 files changed, 8 insertions(+), 8 deletions(-)gatchat/gatio.c +0
-0@@ -91,6 +91,10 @@ static gboolean received_data(GIOChannel *channel, GIOCondition cond, if (cond & G_IO_NVAL) return FALSE;
Quoted 8 lines
gatchat/gatio.c static gboolean received_data(GIOChannel *channel, GIOCondition cond, · line 97
Quoted 4 lines
gatchat/gatio.c +4
-0+/* if nobody wants this data, leave it alone */ +if (io->read_handler ==NULL ) +return TRUE; +This is a really bad idea, we should never have cases where the read handler is unset, and if we do we should end up closing the socket when the buffer is full.
In your implementation of g_at_chat_suspend, you set the read handler to NULL.
Hi Kristen,
Quoted 29 lines
On Mon, 10 May 2010 09:11:52 -0500
Quoted 17 lines · Denis Kenzior
Hi Kristen,
Quoted 15 lines
If no
read_handlerspecified, leave data alone in case someone else wants to read it.--- gatchat/gatio.c | 4 ++++ gatchat/gsmdial.c | 12 ++++-------- 2 files changed, 8 insertions(+), 8 deletions(-)gatchat/gatio.c +0
-0@@ -91,6 +91,10 @@ static gboolean received_data(GIOChannel *channel, GIOCondition cond, if (cond & G_IO_NVAL) return FALSE;Quoted 8 lines
gatchat/gatio.c static gboolean received_data(GIOChannel *channel, GIOCondition cond, · line 97
Quoted 4 lines
gatchat/gatio.c +4
-0+/* if nobody wants this data, leave it alone */ +if (io->read_handler ==NULL ) +return TRUE; +This is a really bad idea, we should never have cases where the read handler is unset, and if we do we should end up closing the socket when the buffer is full.
Quoted 3 lines
In your implementation of
g_at_chat_suspend, you set the read handler to
NULL.
Correct, with the intent that someone else sets the read handler (e.g. g_at_ppp)
Regards,
-Denis
Quoted 31 lines · On Mon, 10 May 2010 12:23:04 -0500 Denis Kenzior <[email protected]> wrote:
Hi Kristen,
Quoted 29 lines
On Mon, 10 May 2010 09:11:52 -0500
Quoted 25 lines · Denis Kenzior
Hi Kristen,
Quoted 12 lines
If no
read_handlerspecified, leave data alone in case someone else wants to read it.--- gatchat/gatio.c | 4 ++++ gatchat/gsmdial.c | 12 ++++-------- 2 files changed, 8 insertions(+), 8 deletions(-)gatchat/gatio.c +0
-0gatchat/gatio.c static gboolean received_data(GIOChannel *channel, · line 97
Quoted 7 lines
gatchat/gatio.c +4
-0@@ -91,6 +91,10 @@ static gboolean received_data(GIOChannel *channel, GIOCondition cond, if (cond & G_IO_NVAL) return FALSE; +/* if nobody wants this data, leave it alone */ +if (io->read_handler ==NULL ) +return TRUE; +This is a really bad idea, we should never have cases where the read handler is unset, and if we do we should end up closing the socket when the buffer is full.
Quoted 6 lines
Quoted 3 lines
In your implementation of
g_at_chat_suspend, you set the read handler to
NULL.Correct, with the intent that someone else sets the read handler (e.g.
g_at_ppp)
What if you receive data after the read handler is set to NULL, but before someone else sets the read handler? shouldn't we safe guard against that?
Hi Kristen,
Quoted 34 lines
On Mon, 10 May 2010 12:23:04 -0500
Quoted 31 lines · Denis Kenzior
Hi Kristen,
Quoted 29 lines
On Mon, 10 May 2010 09:11:52 -0500
Quoted 25 lines · Denis Kenzior
Hi Kristen,
Quoted 12 lines
If no
read_handlerspecified, leave data alone in case someone else wants to read it.--- gatchat/gatio.c | 4 ++++ gatchat/gsmdial.c | 12 ++++-------- 2 files changed, 8 insertions(+), 8 deletions(-)gatchat/gatio.c +0
-0gatchat/gatio.c static gboolean received_data(GIOChannel · line 97
Quoted 7 lines
gatchat/gatio.c +4
-0@@ -91,6 +91,10 @@ static gboolean received_data(GIOChannel *channel, GIOCondition cond, if (cond & G_IO_NVAL)return FALSE; +/* if nobody wants this data, leave it alone */ +if (io->read_handler ==NULL ) +return TRUE; +This is a really bad idea, we should never have cases where the read handler is unset, and if we do we should end up closing the socket when the buffer is full.
Quoted 9 lines
Quoted 5 lines
Quoted 2 lines
In your implementation of
g_at_chat_suspend, you set the read handler to NULL.Correct, with the intent that someone else sets the read handler (e.g.
g_at_ppp)What if you receive data after the read handler is set to NULL, but before someone else sets the read handler? shouldn't we safe guard against that?
We do safe guard, once the buffer runs out of space we close the GAtIO. However, letting GAtIO enter the event-loop (since only then the io watch will fire) without a read handler is a user error. Having an active GAtIO without a read handler is useless...
Regards,
-Denis