[Spice-devel] [spice-server 3/3] char-device: Remove CharDevice::remove_client()

[Frediano Ziglio]

> 
> RedCharDevice subclasses are always implementing it by calling
> red_channel_client_shutdown().
> 
> The only exception is RedStreamDevice. The various codepaths leading
> to a ::remove_client() call are not going to be hit as it does not have
> an associated channel client (it does not call red_char_device_client_add),
> andi t returns NULL from its read_one_msg_from_device implementation so
> red_char_device_send_msg_to_clients() does not get called.
> 
> Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>

Spagnetti code?

RedCharDevice was designed to not depends on channels, it got
the header just because it needs to access to event scheduler
which was accessible through global variables/functions.

> ---
>  server/char-device.c       | 17 +++++++----------
>  server/char-device.h       |  5 -----
>  server/red-stream-device.c |  6 ------
>  server/reds.c              |  7 -------
>  server/smartcard.c         | 11 -----------
>  server/spicevmc.c          | 13 -------------
>  6 files changed, 7 insertions(+), 52 deletions(-)
> 
> diff --git a/server/char-device.c b/server/char-device.c
> index c86c3a2f0..d2fe3b62c 100644
> --- a/server/char-device.c
> +++ b/server/char-device.c
> @@ -140,14 +140,6 @@ red_char_device_on_free_self_token(RedCharDevice *dev)
>     }
>  }
>  
> -static void
> -red_char_device_remove_client(RedCharDevice *dev, RedClient *client)
> -{
> -   RedCharDeviceClass *klass = RED_CHAR_DEVICE_GET_CLASS(dev);
> -
> -   klass->remove_client(dev, client);
> -}
> -
>  static void red_char_device_write_buffer_free(RedCharDeviceWriteBuffer *buf)
>  {
>      if (buf == NULL)
> @@ -216,10 +208,15 @@ static void red_char_device_client_free(RedCharDevice
> *dev,
>      g_free(dev_client);
>  }
>  
> +
> +/* This method is called if it is recommended to remove the client
> + * due to slow flow or due to some other error.
> + * The called instance should disconnect the client, or at least the
> + * corresponding channel */
>  static void red_char_device_handle_client_overflow(RedCharDeviceClient
>  *dev_client)
>  {
> -    RedCharDevice *dev = dev_client->dev;
> -    red_char_device_remove_client(dev, dev_client->client);
> +    g_return_if_fail(dev_client != NULL);
> +    red_channel_client_shutdown(RED_CHANNEL_CLIENT(dev_client));
>  }
>  
>  static RedCharDeviceClient *red_char_device_client_find(RedCharDevice *dev,
> diff --git a/server/char-device.h b/server/char-device.h
> index 893d3e4b1..1220e080c 100644
> --- a/server/char-device.h
> +++ b/server/char-device.h
> @@ -74,11 +74,6 @@ struct RedCharDeviceClass
>       * has been completely written to it */
>      void (*on_free_self_token)(RedCharDevice *self);
>  
> -    /* This cb is called if it is recommended to remove the client
> -     * due to slow flow or due to some other error.
> -     * The called instance should disconnect the client, or at least the
> corresponding channel */
> -    void (*remove_client)(RedCharDevice *self, RedClient *client);
> -
>      /* This cb is called when device receives an event */
>      void (*port_event)(RedCharDevice *self, uint8_t event);
>  };
> diff --git a/server/red-stream-device.c b/server/red-stream-device.c
> index b9e0827af..86a308ba1 100644
> --- a/server/red-stream-device.c
> +++ b/server/red-stream-device.c
> @@ -573,11 +573,6 @@ stream_device_send_msg_to_client(RedCharDevice *self,
> RedPipeItem *msg, RedClien
>  {
>  }
>  
> -static void
> -stream_device_remove_client(RedCharDevice *self, RedClient *client)
> -{
> -}
> -
>  static void
>  stream_device_stream_start(void *opaque, StreamMsgStartStop *start,
>                             StreamChannel *stream_channel G_GNUC_UNUSED)
> @@ -789,7 +784,6 @@ stream_device_class_init(StreamDeviceClass *klass)
>  
>      char_dev_class->read_one_msg_from_device =
>      stream_device_read_msg_from_dev;
>      char_dev_class->send_msg_to_client = stream_device_send_msg_to_client;
> -    char_dev_class->remove_client = stream_device_remove_client;
>      char_dev_class->port_event = stream_device_port_event;
>  }
>  
> diff --git a/server/reds.c b/server/reds.c
> index 24047bda7..32bb1ddfe 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -1055,12 +1055,6 @@ static void vdi_port_on_free_self_token(RedCharDevice
> *self)
>      }
>  }
>  
> -static void vdi_port_remove_client(RedCharDevice *self,
> -                                   RedClient *client)
> -{
> -
> red_channel_client_shutdown(RED_CHANNEL_CLIENT(red_client_get_main(client)));
> -}
> -
>  /****************************************************************************/
>  
>  int reds_has_vdagent(RedsState *reds)
> @@ -4631,7 +4625,6 @@
> red_char_device_vdi_port_class_init(RedCharDeviceVDIPortClass *klass)
>      char_dev_class->read_one_msg_from_device =
>      vdi_port_read_one_msg_from_device;
>      char_dev_class->send_msg_to_client = vdi_port_send_msg_to_client;
>      char_dev_class->send_tokens_to_client = vdi_port_send_tokens_to_client;
> -    char_dev_class->remove_client = vdi_port_remove_client;
>      char_dev_class->on_free_self_token = vdi_port_on_free_self_token;
>  }
>  
> diff --git a/server/smartcard.c b/server/smartcard.c
> index c08228271..c67081fda 100644
> --- a/server/smartcard.c
> +++ b/server/smartcard.c
> @@ -186,16 +186,6 @@ static void smartcard_send_msg_to_client(RedCharDevice
> *self,
>      red_channel_client_pipe_add_push(rcc, msg);
>  }
>  
> -static void smartcard_remove_client(RedCharDevice *self, RedClient *client)
> -{
> -    RedCharDeviceSmartcard *dev = RED_CHAR_DEVICE_SMARTCARD(self);
> -    RedChannelClient *rcc = RED_CHANNEL_CLIENT(dev->priv->scc);
> -
> -    spice_assert(dev->priv->scc &&
> -                 red_channel_client_get_client(rcc) == client);
> -    red_channel_client_shutdown(rcc);
> -}
> -
>  RedMsgItem
>  *smartcard_char_device_on_message_from_device(RedCharDeviceSmartcard *dev,
>                                                           VSCMsgHeader
>                                                           *vheader)
>  {
> @@ -606,7 +596,6 @@
> red_char_device_smartcard_class_init(RedCharDeviceSmartcardClass *klass)
>  
>      char_dev_class->read_one_msg_from_device =
>      smartcard_read_msg_from_device;
>      char_dev_class->send_msg_to_client = smartcard_send_msg_to_client;
> -    char_dev_class->remove_client = smartcard_remove_client;
>  }
>  
>  static void
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index 51550c1a0..8e4712a55 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -420,18 +420,6 @@ static void spicevmc_port_send_event(RedChannelClient
> *rcc, uint8_t event)
>      red_channel_client_pipe_add_push(rcc, &item->base);
>  }
>  
> -static void spicevmc_char_dev_remove_client(RedCharDevice *self,
> -                                            RedClient *client)
> -{
> -    RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(self);
> -    RedVmcChannel *channel = RED_VMC_CHANNEL(vmc->channel);
> -
> -    spice_assert(channel->rcc &&
> -                 red_channel_client_get_client(channel->rcc) == client);
> -
> -    red_channel_client_shutdown(channel->rcc);
> -}
> -
>  static void spicevmc_red_channel_client_on_disconnect(RedChannelClient *rcc)
>  {
>      RedVmcChannel *channel;
> @@ -895,7 +883,6 @@
> red_char_device_spicevmc_class_init(RedCharDeviceSpiceVmcClass *klass)
>  
>      char_dev_class->read_one_msg_from_device =
>      spicevmc_chardev_read_msg_from_dev;
>      char_dev_class->send_msg_to_client =
>      spicevmc_chardev_send_msg_to_client;
> -    char_dev_class->remove_client = spicevmc_char_dev_remove_client;
>      char_dev_class->port_event = spicevmc_port_event;
>  
>      g_object_class_install_property(object_class,

Frediano