[PATCH] gatmux: fix channel remove on error.

[PATCH] gatmux: fix channel remove on error.

[Antoine Aubert]

[-- Attachment #1: Type: text/plain, Size: 1232 bytes --]

In case of invalid IO, read_watch is not reset. Fix crash on destroy
gatmux.

Signed-off-by: Antoine Aubert <a.aubert@overkiz.com>
---
 gatchat/gatmux.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/gatchat/gatmux.c b/gatchat/gatmux.c
index 9beeece..896ddff 100644
--- a/gatchat/gatmux.c
+++ b/gatchat/gatmux.c
@@ -186,8 +186,10 @@ static gboolean received_data(GIOChannel *channel, GIOCondition cond,
 	GIOStatus status;
 	gsize bytes_read;
 
-	if (cond & G_IO_NVAL)
+	if (cond & G_IO_NVAL) {
+		mux->read_watch = 0;
 		return FALSE;
+	}
 
 	debug(mux, "received data");
 
@@ -223,14 +225,20 @@ static gboolean received_data(GIOChannel *channel, GIOCondition cond,
 		}
 	}
 
-	if (cond & (G_IO_HUP | G_IO_ERR))
+	if (cond & (G_IO_HUP | G_IO_ERR)) {
+		mux->read_watch = 0;
 		return FALSE;
+	}
 
-	if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN)
+	if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN) {
+		mux->read_watch = 0;
 		return FALSE;
+	}
 
-	if (mux->buf_used == sizeof(mux->buf))
+	if (mux->buf_used == sizeof(mux->buf)) {
+		mux->read_watch = 0;
 		return FALSE;
+	}
 
 	return TRUE;
 }
-- 
2.7.4


[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1489 bytes --]

Re: [PATCH] gatmux: fix channel remove on error.

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 2109 bytes --]

Hi Antoine,

On 10/04/2016 02:42 AM, Antoine Aubert wrote:
> In case of invalid IO, read_watch is not reset. Fix crash on destroy
> gatmux.
>
> Signed-off-by: Antoine Aubert<a.aubert@overkiz.com>

We don't use Signed-off-by here.  See HACKING, Submitting Patches section.

> ---
>   gatchat/gatmux.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/gatchat/gatmux.c b/gatchat/gatmux.c
> index 9beeece..896ddff 100644
> --- a/gatchat/gatmux.c
> +++ b/gatchat/gatmux.c
> @@ -186,8 +186,10 @@ static gboolean received_data(GIOChannel *channel, GIOCondition cond,
>   	GIOStatus status;
>   	gsize bytes_read;
>
> -	if (cond & G_IO_NVAL)
> +	if (cond & G_IO_NVAL) {
> +		mux->read_watch = 0;
>   		return FALSE;
> +	}

Good catch on this one.  However it might be easier solved by using a 
watch destroy notify function.  E.g. in g_at_mux_start():

         mux->read_watch = g_io_add_watch_full(mux->channel, 
G_PRIORITY_DEFAULT,
                                 G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
                                                 received_data, mux, NULL);

The last NULL parameter should be set to a new function that resets the 
mux->read_watch to 0.  Similar to write_watcher_destroy_notify.

>
>   	debug(mux, "received data");
>
> @@ -223,14 +225,20 @@ static gboolean received_data(GIOChannel *channel, GIOCondition cond,
>   		}
>   	}
>
> -	if (cond & (G_IO_HUP | G_IO_ERR))
> +	if (cond & (G_IO_HUP | G_IO_ERR)) {
> +		mux->read_watch = 0;
>   		return FALSE;
> +	}
>
> -	if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN)
> +	if (status != G_IO_STATUS_NORMAL && status != G_IO_STATUS_AGAIN) {
> +		mux->read_watch = 0;
>   		return FALSE;
> +	}
>
> -	if (mux->buf_used == sizeof(mux->buf))
> +	if (mux->buf_used == sizeof(mux->buf)) {
> +		mux->read_watch = 0;
>   		return FALSE;
> +	}

That way all these redundant statements are handled by the destroy notify.

>
>   	return TRUE;
>   }
> --
> 2.7.4
>

Care to resubmit?

Regards,
-Denis

[PATCH] gatmux: fix read channel remove on error.

[Antoine Aubert]

[-- Attachment #1: Type: text/plain, Size: 892 bytes --]

In case of invalid IO, read_watch is not reset. This fix crash on destroy gatmux.
---
 gatchat/gatmux.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/gatchat/gatmux.c b/gatchat/gatmux.c
index 9beeece..074e340 100644
--- a/gatchat/gatmux.c
+++ b/gatchat/gatmux.c
@@ -598,6 +598,13 @@ void g_at_mux_unref(GAtMux *mux)
 	}
 }
 
+static void read_watcher_destroy_notify(gpointer user_data)
+{
+	GAtMux *mux = user_data;
+
+	mux->read_watch = 0;
+}
+
 gboolean g_at_mux_start(GAtMux *mux)
 {
 	if (mux->channel == NULL)
@@ -611,7 +618,7 @@ gboolean g_at_mux_start(GAtMux *mux)
 
 	mux->read_watch = g_io_add_watch_full(mux->channel, G_PRIORITY_DEFAULT,
 				G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL,
-						received_data, mux, NULL);
+						received_data, mux, read_watcher_destroy_notify);
 
 	mux->shutdown = FALSE;
 
-- 
2.7.4

Re: [PATCH] gatmux: fix read channel remove on error.

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 287 bytes --]

Hi Antoine,

On 10/05/2016 01:42 AM, Antoine Aubert wrote:
> In case of invalid IO, read_watch is not reset. This fix crash on destroy gatmux.
> ---
>   gatchat/gatmux.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>

Applied, thanks.

Regards,
-Denis