[Spice-devel] [spice-server v2 2/6] net: Introduce red_socket_set_non_blocking() helper

[Frediano Ziglio]

> 
> This allows to move some low-level code out of reds.c
> 

Wonderful! Too many stuff in this file.

> Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> ---
>  server/net-utils.c | 17 +++++++++++++++++
>  server/net-utils.h |  1 +
>  server/reds.c      | 18 +++++-------------
>  3 files changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/server/net-utils.c b/server/net-utils.c
> index e2c4853..ce409be 100644
> --- a/server/net-utils.c
> +++ b/server/net-utils.c
> @@ -45,3 +45,20 @@ bool red_socket_set_no_delay(int fd, bool no_delay)
>  
>      return true;
>  }
> +
> +bool red_socket_set_non_blocking(int fd, bool non_blocking)
> +{
> +    int flags;
> +
> +    if ((flags = fcntl(fd, F_GETFL)) == -1) {
> +        spice_warning("fnctl(F_GETFL) failed, %s", strerror(errno));
> +        return false;
> +    }
> +
> +    if (fcntl(fd, F_SETFL, flags | O_NONBLOCK) == -1) {
> +        spice_warning("fnctl(F_SETFL) failed, %s", strerror(errno));
> +        return false;
> +    }
> +
> +    return true;
> +}
> diff --git a/server/net-utils.h b/server/net-utils.h
> index d49ebc4..aed6956 100644
> --- a/server/net-utils.h
> +++ b/server/net-utils.h
> @@ -19,5 +19,6 @@
>  #define _H_RED_NET_UTILS
>  
>  bool red_socket_set_no_delay(int fd, bool no_delay);
> +bool red_socket_set_non_blocking(int fd, bool non_blocking);
>  
>  #endif
> diff --git a/server/reds.c b/server/reds.c
> index e3e2655..12c797b 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -31,8 +31,6 @@
>  #include <limits.h>
>  #include <pthread.h>
>  #include <sys/mman.h>
> -#include <fcntl.h>
> -#include <errno.h>
>  #include <ctype.h>
>  
>  #include <openssl/err.h>
> @@ -2405,20 +2403,14 @@ static bool reds_init_keepalive(int socket)
>  static RedLinkInfo *reds_init_client_connection(RedsState *reds, int socket)
>  {
>      RedLinkInfo *link;
> -    int flags;
> -
> -    if ((flags = fcntl(socket, F_GETFL)) == -1) {
> -        spice_warning("accept failed, %s", strerror(errno));
> -        goto error;
> -    }
> -
> -    if (fcntl(socket, F_SETFL, flags | O_NONBLOCK) == -1) {
> -        spice_warning("accept failed, %s", strerror(errno));
> -        goto error;
> -    }
>  
>      reds_init_keepalive(socket);
>  
> +    if (!red_socket_set_non_blocking(socket, TRUE))
> +    {

bracket should be with the if.
I would use true.

> +       goto error;
> +    }
> +
>      link = spice_new0(RedLinkInfo, 1);
>      link->reds = reds;
>      link->stream = reds_stream_new(reds, socket);

Beside that,

Acked-by: Frediano Ziglio <fziglio at redhat.com>

Frediano