[Spice-devel] [PATCH spice-server] char-device: Make RedClient an opaque structure again

[Frediano Ziglio]

> 
> On Tue, Mar 12, 2019 at 05:58:40AM -0400, Frediano Ziglio wrote:
> > ping
> > 
> > > 
> > > RedClient was an opaque structure for RedCharDevice.
> > > It started to be used when RedsState started to contain all
> > > the global state.
> > > Make it opaque again.
> 
> It seems we don't put the same meaning to 'opaque'. For me, RedClient is
> already an opaque structure from a char-device.c perspective as it's not
> dereferencing RedClient * (ie it does not know/care which fields are
> defined in struct RedClient).
> 

Maybe "generic" ? Anything ?
The idea was to have a "RedCharDeviceClientOpaque" pointer with
RedCharDeviceClientOpaque as a typename so you could have something
like

#ifndef RedCharDeviceClientOpaque
typedef struct RedCharDeviceClientOpaque RedCharDeviceClientOpaque;
#define RedCharDeviceClientOpaque RedCharDeviceClientOpaque
#endif

in char-device.h

> This commit goes further than that as it removes any calls to
> red_client_* API. char-device.c still cares that it is handling a
> pointer to a RedClient * (as opposed to a gpointer client_handle which
> could be anything). Do you have some longer term goal with respect to
> RedClient and RedCharDevice, or is this just an isolated patch?
> 

I have a follow up that uses RedChannelClient as RedCharDeviceClientOpaque
instead.
But I still not decided the future plan but this way I can change to
RedChannel without touching RedCharDevice. There are parts of the code
handling multi-channel while other parts are broken in this respect.
Personally I think multi-channel does not make sense for this object.
Also token handling is broken (unlimited queue) and only used for the
agent.
I started splitting the agent which seemed to make more sense.
It is getting into shape and I think it would help defining what to do
with RedCharDevice. But time is scarce and progress is slow.

> Christophe
> 
> 
> > > 
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > >  server/char-device.c | 16 +++++++---------
> > >  1 file changed, 7 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/server/char-device.c b/server/char-device.c
> > > index 040b91147..465c1a125 100644
> > > --- a/server/char-device.c
> > > +++ b/server/char-device.c
> > > @@ -22,8 +22,8 @@
> > >  
> > >  #include <config.h>
> > >  #include <inttypes.h>
> > > +
> > >  #include "char-device.h"
> > > -#include "red-client.h"
> > >  #include "reds.h"
> > >  #include "glib-compat.h"
> > >  
> > > @@ -703,11 +703,10 @@ void red_char_device_destroy(RedCharDevice
> > > *char_dev)
> > >      g_object_unref(char_dev);
> > >  }
> > >  
> > > -static RedCharDeviceClient *red_char_device_client_new(RedClient
> > > *client,
> > > -                                                       int
> > > do_flow_control,
> > > -                                                       uint32_t
> > > max_send_queue_size,
> > > -                                                       uint32_t
> > > num_client_tokens,
> > > -                                                       uint32_t
> > > num_send_tokens)
> > > +static RedCharDeviceClient *
> > > +red_char_device_client_new(RedsState *reds, RedClient *client,
> > > +                           int do_flow_control, uint32_t
> > > max_send_queue_size,
> > > +                           uint32_t num_client_tokens, uint32_t
> > > num_send_tokens)
> > >  {
> > >      RedCharDeviceClient *dev_client;
> > >  
> > > @@ -717,8 +716,6 @@ static RedCharDeviceClient
> > > *red_char_device_client_new(RedClient *client,
> > >      dev_client->max_send_queue_size = max_send_queue_size;
> > >      dev_client->do_flow_control = do_flow_control;
> > >      if (do_flow_control) {
> > > -        RedsState *reds = red_client_get_server(client);
> > > -
> > >          dev_client->wait_for_tokens_timer =
> > >              reds_core_timer_add(reds,
> > >              device_client_wait_for_tokens_timeout,
> > >                                  dev_client);
> > > @@ -757,7 +754,8 @@ bool red_char_device_client_add(RedCharDevice *dev,
> > >      dev->priv->wait_for_migrate_data = wait_for_migrate_data;
> > >  
> > >      spice_debug("char device %p, client %p", dev, client);
> > > -    dev_client = red_char_device_client_new(client, do_flow_control,
> > > +    dev_client = red_char_device_client_new(dev->priv->reds, client,
> > > +                                            do_flow_control,
> > >                                              max_send_queue_size,
> > >                                              num_client_tokens,
> > >                                              num_send_tokens);
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
>