>
> 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.
> ---
> server/reds.c | 47 +++++++++++++++++++++++++++++------------------
> 1 file changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/server/reds.c b/server/reds.c
> index a31ed4e96..829158ca3 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -2584,19 +2584,36 @@ 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) {
> reds->listen_socket = reds_init_socket(reds->config->spice_addr,
> reds->config->spice_port, reds->config->spice_family);
> if (-1 == reds->listen_socket) {
> - return -1;
> + goto error;
> }
> reds->listen_watch = reds_core_watch_add(reds, reds->listen_socket,
> SPICE_WATCH_EVENT_READ,
> reds_accept, reds);
> if (reds->listen_watch == NULL) {
> - spice_warning("set fd handle failed");
> - return -1;
> + goto error;
> }
> }
>
> @@ -2604,14 +2621,13 @@ static int reds_init_net(RedsState *reds)
> reds->secure_listen_socket =
> reds_init_socket(reds->config->spice_addr,
> reds->config->spice_secure_port,
> reds->config->spice_family);
> if (-1 == reds->secure_listen_socket) {
> - return -1;
> + goto error;
> }
> reds->secure_listen_watch = reds_core_watch_add(reds,
> reds->secure_listen_socket,
> SPICE_WATCH_EVENT_READ,
> reds_accept_ssl_connection,
> reds);
> if (reds->secure_listen_watch == NULL) {
> - spice_warning("set fd handle failed");
> - return -1;
> + goto error;
> }
> }
>
> @@ -2621,11 +2637,15 @@ 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;
> + reds->listen_socket = -1;
Is this necessary taking into account the check in reds_cleanup_net
> + goto error;
> }
> }
> return 0;
> +
> +error:
> + reds_cleanup_net(reds);
Maybe this call can be in the err label inside do_spice_init?
This would reduce reds_init_net changes and improve do_spice_init too.
> + return -1;
> }
>
> static int load_dh_params(SSL_CTX *ctx, char *file)
> @@ -3632,16 +3652,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);
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel