[Spice-devel] [PATCH 3/4] hide CursorChannelClient implementation details

[Frediano Ziglio]

> 
> On Fri, 2016-05-13 at 10:16 +0100, Frediano Ziglio wrote:
> > The existence of this class can be hidden to user of CursorChannel class
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/cursor-channel.c | 15 +++++++++++----
> >  server/cursor-channel.h |  7 ++-----
> >  server/red-worker.c     |  4 ++--
> >  3 files changed, 15 insertions(+), 11 deletions(-)
> > 
> > diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> > index 66f0181..688bab4 100644
> > --- a/server/cursor-channel.c
> > +++ b/server/cursor-channel.c
> > @@ -31,6 +31,8 @@
> >  #define CURSOR_CACHE_HASH_MASK (CURSOR_CACHE_HASH_SIZE - 1)
> >  #define CURSOR_CACHE_HASH_KEY(id) ((id) & CURSOR_CACHE_HASH_MASK)
> >  
> > +typedef struct CursorChannelClient CursorChannelClient;
> > +
> >  enum {
> >      RED_PIPE_ITEM_TYPE_CURSOR = RED_PIPE_ITEM_TYPE_COMMON_LAST,
> >      RED_PIPE_ITEM_TYPE_CURSOR_INIT,
> > @@ -439,9 +441,9 @@ CursorChannel* cursor_channel_new(RedWorker *worker)
> >      return cursor_channel;
> >  }
> >  
> > -void cursor_channel_client_migrate(CursorChannelClient* client)
> > +void cursor_channel_client_migrate(RedChannelClient *rcc)
> >  {
> > -    RedChannelClient *rcc;
> > +    CursorChannelClient* client = (CursorChannelClient*)rcc;
> >  
> >      spice_return_if_fail(client);
> >      rcc = RED_CHANNEL_CLIENT(client);
> > @@ -548,7 +550,7 @@ void cursor_channel_reset(CursorChannel *cursor)
> >      }
> >  }
> >  
> > -void cursor_channel_init(CursorChannel *cursor, CursorChannelClient
> > *client)
> > +static void cursor_channel_init_client(CursorChannel *cursor,
> > CursorChannelClient *client)
> >  {
> >      spice_return_if_fail(cursor);
> >  
> > @@ -565,6 +567,11 @@ void cursor_channel_init(CursorChannel *cursor,
> > CursorChannelClient *client)
> >          red_channel_pipes_add_type(RED_CHANNEL(cursor),
> > RED_PIPE_ITEM_TYPE_CURSOR_INIT);
> >  }
> >  
> > +void cursor_channel_init(CursorChannel *cursor)
> > +{
> > +    cursor_channel_init_client(cursor, NULL);
> > +}
> > +
> >  void cursor_channel_set_mouse_mode(CursorChannel *cursor, uint32_t mode)
> >  {
> >      spice_return_if_fail(cursor);
> > @@ -592,5 +599,5 @@ void cursor_channel_connect(CursorChannel *cursor,
> > RedClient *client, RedsStream
> >      red_channel_client_ack_zero_messages_window(rcc);
> >      red_channel_client_push_set_ack(rcc);
> >  
> > -    cursor_channel_init(cursor, ccc);
> > +    cursor_channel_init_client(cursor, ccc);
> >  }
> > diff --git a/server/cursor-channel.h b/server/cursor-channel.h
> > index 6bba822..6c89bc3 100644
> > --- a/server/cursor-channel.h
> > +++ b/server/cursor-channel.h
> > @@ -24,14 +24,11 @@
> >  #include "red-parse-qxl.h"
> >  
> >  typedef struct CursorChannel CursorChannel;
> > -typedef struct CursorChannelClient CursorChannelClient;
> > -
> > -#define CURSOR_CHANNEL_CLIENT(Client) ((CursorChannelClient*)(Client))
> >  
> >  CursorChannel*       cursor_channel_new         (RedWorker *worker);
> >  void                 cursor_channel_disconnect  (CursorChannel
> > *cursor_channel);
> >  void                 cursor_channel_reset       (CursorChannel *cursor);
> > -void                 cursor_channel_init        (CursorChannel *cursor,
> > CursorChannelClient* client);
> > +void                 cursor_channel_init        (CursorChannel *cursor);
> >  void                 cursor_channel_process_cmd (CursorChannel *cursor,
> > RedCursorCmd *cursor_cmd);
> >  void                 cursor_channel_set_mouse_mode(CursorChannel *cursor,
> > uint32_t mode);
> >  void                 cursor_channel_connect     (CursorChannel *cursor,
> > RedClient *client,
> > @@ -40,6 +37,6 @@ void                 cursor_channel_connect
> >  (CursorChannel *cursor, RedClien
> >                                                   uint32_t *common_caps,
> >                                                   int
> > num_common_caps,
> >                                                   uint32_t *caps, int
> > num_caps);
> >  
> > -void                 cursor_channel_client_migrate(CursorChannelClient*
> > client);
> > +void                 cursor_channel_client_migrate(RedChannelClient
> > *client);
> 
> Seems like this may be a potential future RedChannelClient vfunc? Not really
> relevant to this patch though.
> 

It's already a callback (so a vfunc).

Actually it's quite relevant for this patch.
This patch is trying to quite strongly demonstrate that users of a channel
does not need to know even the EXISTENCE of a client class.
This was not well defined in the source and is getting to be hidden by GObject
patches (not by GObject needs, just was easier to retain this knowledge).
The simple needs to include a "cursor-channel-client.h" in "cursor-channel.h"
is now explicitly a mistake. This is valid in mostly RedChannel hierarchies
(I'm trying to define it even for DisplayChannel but code is quite entangled,
but I'm getting some patches).
The derived RedChannel and relative derived RedChannelClient are quite bound
together and some stuff (like the pipe items between them) define the
relationship between them (and don't need to be known by users); this is
the reason I suggested to move Item declarations (like 
RED_PIPE_ITEM_TYPE_* in a11b785f191d2381932f8c1bb6281539f2afe483) to
client header instead of main one I think I suggested the move for another
class).

About the vfunc... I think it's better to explain this in reply of 4/4 which
is trying to document this.

> >  
> >  #endif /* CURSOR_CHANNEL_H_ */
> > diff --git a/server/red-worker.c b/server/red-worker.c
> > index f6d626b..ce65444 100644
> > --- a/server/red-worker.c
> > +++ b/server/red-worker.c
> > @@ -687,7 +687,7 @@ static void dev_create_primary_surface(RedWorker
> > *worker,
> > uint32_t surface_id,
> >          red_channel_push(&worker->display_channel->common.base);
> >      }
> >  
> > -    cursor_channel_init(worker->cursor_channel, NULL);
> > +    cursor_channel_init(worker->cursor_channel);
> >  }
> >  
> >  static void handle_dev_create_primary_surface(void *opaque, void *payload)
> > @@ -1000,7 +1000,7 @@ static void handle_dev_cursor_migrate(void *opaque,
> > void
> > *payload)
> >      RedChannelClient *rcc = msg->rcc;
> >  
> >      spice_info("migrate cursor client");
> > -    cursor_channel_client_migrate(CURSOR_CHANNEL_CLIENT(rcc));
> > +    cursor_channel_client_migrate(rcc);
> >  }
> >  
> >  static void handle_dev_set_compression(void *opaque, void *payload)
> 
> Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
> 
> 
> 

Frediano