[Spice-devel] [spice-server v3] reds: Close sockets when failing to watch them

[Uri Lublin]

On 03/08/2018 03:23 PM, Christophe Fergeau wrote:
> Currently if we fail to set up the watch waiting for accept() to be
> called on the socket, we still keep the network socket(s) open even if we
> are not going to be able to use it. This commit makes sure it's closed a
> set to -1 when such a failure occurs rather than having a half
> initialized spice-server instance.

Ack.

I see below you removed some log-messages.
Perhaps it would be better to just change their messages.

Uri.


> ---
>   server/reds.c | 33 ++++++++++++++++++++-------------
>   1 file changed, 20 insertions(+), 13 deletions(-)
> 
> diff --git a/server/reds.c b/server/reds.c
> index a31ed4e96..8ccb5e0e4 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -2584,6 +2584,24 @@ void reds_set_client_mm_time_latency(RedsState *reds, RedClient *client, uint32_
>       }
>   }
>   
> +static void reds_cleanup_net(SpiceServer *reds)
> +{
> +    if (reds->listen_socket != -1) {
> +       reds_core_watch_remove(reds, reds->listen_watch);
> +       if (reds->config->spice_listen_socket_fd != reds->listen_socket) {
> +          close(reds->listen_socket);
> +       }
> +       reds->listen_watch = NULL;
> +       reds->listen_socket = -1;
> +    }
> +    if (reds->secure_listen_socket != -1) {
> +       reds_core_watch_remove(reds, reds->secure_listen_watch);
> +       close(reds->secure_listen_socket);
> +       reds->secure_listen_watch = NULL;
> +       reds->secure_listen_socket = -1;
> +    }
> +}
> +
>   static int reds_init_net(RedsState *reds)
>   {
>       if (reds->config->spice_port != -1 || reds->config->spice_family == AF_UNIX) {
> @@ -2595,7 +2613,6 @@ static int reds_init_net(RedsState *reds)
>                                                    SPICE_WATCH_EVENT_READ,
>                                                    reds_accept, reds);
>           if (reds->listen_watch == NULL) {
> -            spice_warning("set fd handle failed");
>               return -1;
>           }
>       }
> @@ -2610,7 +2627,6 @@ static int reds_init_net(RedsState *reds)
>                                                           SPICE_WATCH_EVENT_READ,
>                                                           reds_accept_ssl_connection, reds);
>           if (reds->secure_listen_watch == NULL) {
> -            spice_warning("set fd handle failed");
>               return -1;
>           }
>       }
> @@ -2621,7 +2637,6 @@ static int reds_init_net(RedsState *reds)
>                                                    SPICE_WATCH_EVENT_READ,
>                                                    reds_accept, reds);
>           if (reds->listen_watch == NULL) {
> -            spice_warning("set fd handle failed");
>               return -1;
>           }
>       }
> @@ -3373,6 +3388,7 @@ static int do_spice_init(RedsState *reds, SpiceCoreInterface *core_interface)
>       return 0;
>   
>   err:
> +    reds_cleanup_net(reds);
>       return -1;
>   }
>   
> @@ -3632,16 +3648,7 @@ SPICE_GNUC_VISIBLE void spice_server_destroy(SpiceServer *reds)
>       if (reds->main_dispatcher) {
>           g_object_unref(reds->main_dispatcher);
>       }
> -    if (reds->listen_socket != -1) {
> -       reds_core_watch_remove(reds, reds->listen_watch);
> -       if (reds->config->spice_listen_socket_fd != reds->listen_socket) {
> -          close(reds->listen_socket);
> -       }
> -    }
> -    if (reds->secure_listen_socket != -1) {
> -       reds_core_watch_remove(reds, reds->secure_listen_watch);
> -       close(reds->secure_listen_socket);
> -    }
> +    reds_cleanup_net(reds);
>       g_clear_object(&reds->agent_dev);
>       spice_buffer_free(&reds->client_monitors_config);
>       red_record_unref(reds->record);
>