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

Submitted by Christophe Fergeau on Feb. 7, 2019, 1:02 p.m.

Details

Message ID 20190207130252.12721-3-cfergeau@redhat.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Christophe Fergeau Feb. 7, 2019, 1:02 p.m.
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@redhat.com>
---
 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(-)

Patch hide | download patch | download mbox

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,

Comments

> 
> 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@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