Re: [spice-gtk v4 3/6] spice_channel_read_wire: prefer while(TRUE) instead of goto

[Christophe Fergeau <cfergeau@xxxxxxxxxx>]

On Thu, Mar 02, 2017 at 03:08:11PM +0100, Victor Toso wrote:
> From: Victor Toso <me@xxxxxxxxxxxxxx>
> 
> Although this is likely to be a single loop iteration based on the
> implementation of g_coroutine_socket_wait(), using goto to reiterate
> in the code should be avoided based on spice style recommendation.
> 
> This also make it easier to add follow up changes that can increase
> the loop iterations.
> 
> This patch changes:
>  * The reread label with a while(TRUE);
>  * The goto keyword with continue;
> 
> All other changes are only related to new indentation.

In other words, git show -w is:

diff --git a/src/spice-channel.c b/src/spice-channel.c
index 999c615..f2d1b8a 100644
--- a/src/spice-channel.c
+++ b/src/spice-channel.c
@@ -1051,7 +1051,7 @@ static int spice_channel_read_wire(SpiceChannel *channel, void *data, size_t len
     GIOCondition cond;
     gssize ret;

-reread:
+    while (TRUE) {

         if (c->has_error) return 0; /* has_error is set by disconnect(), return no error */

@@ -1061,7 +1061,7 @@ reread:
             if (cond != 0) {
                 // TODO: should use g_pollable_input/output_stream_create_source() ?
                 g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
-            goto reread;
+                continue;
             } else {
                 c->has_error = TRUE;
                 return -errno;
@@ -1075,6 +1075,7 @@ reread:

         return ret;
     }
+}

Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>


> 
> Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> ---
>  src/spice-channel.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index 999c615..f2d1b8a 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -1051,29 +1051,30 @@ static int spice_channel_read_wire(SpiceChannel *channel, void *data, size_t len
>      GIOCondition cond;
>      gssize ret;
>  
> -reread:
> +    while (TRUE) {
>  
> -    if (c->has_error) return 0; /* has_error is set by disconnect(), return no error */
> +        if (c->has_error) return 0; /* has_error is set by disconnect(), return no error */
>  
> -    ret = spice_channel_read_wire_nonblocking(channel, data, len, &cond);
> +        ret = spice_channel_read_wire_nonblocking(channel, data, len, &cond);
>  
> -    if (ret == -1) {
> -        if (cond != 0) {
> -            // TODO: should use g_pollable_input/output_stream_create_source() ?
> -            g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
> -            goto reread;
> -        } else {
> +        if (ret == -1) {
> +            if (cond != 0) {
> +                // TODO: should use g_pollable_input/output_stream_create_source() ?
> +                g_coroutine_socket_wait(&c->coroutine, c->sock, cond);
> +                continue;
> +            } else {
> +                c->has_error = TRUE;
> +                return -errno;
> +            }
> +        }
> +        if (ret == 0) {
> +            CHANNEL_DEBUG(channel, "Closing the connection: spice_channel_read() - ret=0");
>              c->has_error = TRUE;
> -            return -errno;
> +            return 0;
>          }
> -    }
> -    if (ret == 0) {
> -        CHANNEL_DEBUG(channel, "Closing the connection: spice_channel_read() - ret=0");
> -        c->has_error = TRUE;
> -        return 0;
> -    }
>  
> -    return ret;
> +        return ret;
> +    }
>  }
>  
>  #ifdef HAVE_SASL
> -- 
> 2.9.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel