[Spice-devel] [PATCH 05/11] make red_pipe_item_init_full more typesafe

[Frediano Ziglio]

> 
> On Fri, May 20, 2016 at 02:01:43PM +0100, Frediano Ziglio wrote:
> > Use a proper type for free callback
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/cursor-channel.c | 10 ++++++----
> >  server/dcc.c            |  8 +++++---
> >  server/main-channel.c   | 10 ++++++----
> >  server/red-channel.c    |  6 ++++--
> >  server/red-pipe-item.c  |  4 ++--
> >  server/red-pipe-item.h  | 10 +++++++---
> >  server/reds.c           |  8 +++++---
> >  server/smartcard.c      |  5 +++--
> >  server/stream.c         | 15 +++++++++------
> >  9 files changed, 47 insertions(+), 29 deletions(-)
> > 
> > diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> > index 949cbc4..c257654 100644
> > --- a/server/cursor-channel.c
> > +++ b/server/cursor-channel.c
> > @@ -81,7 +81,7 @@ struct CursorChannelClient {
> >  #include "cache-item.tmpl.c"
> >  #undef CLIENT_CURSOR_CACHE
> >  
> > -static void cursor_pipe_item_free(RedCursorPipeItem *pipe_item);
> > +static red_pipe_item_free_t cursor_pipe_item_free;
> >  
> >  static CursorItem *cursor_item_new(QXLInstance *qxl, RedCursorCmd *cmd)
> >  {
> > @@ -138,7 +138,7 @@ static RedPipeItem
> > *new_cursor_pipe_item(RedChannelClient *rcc, void *data, int
> >      RedCursorPipeItem *item = spice_malloc0(sizeof(RedCursorPipeItem));
> >  
> >      red_pipe_item_init_full(&item->base, RED_PIPE_ITEM_TYPE_CURSOR,
> > -                            (GDestroyNotify)cursor_pipe_item_free);
> > +                            cursor_pipe_item_free);
> >      item->cursor_item = data;
> >      item->cursor_item->refs++;
> >      return &item->base;
> > @@ -204,9 +204,11 @@ void cursor_channel_disconnect(CursorChannel
> > *cursor_channel)
> >  }
> >  
> >  
> > -static void cursor_pipe_item_free(RedCursorPipeItem *pipe_item)
> > +static void cursor_pipe_item_free(RedPipeItem *base)
> >  {
> > -    spice_return_if_fail(pipe_item);
> > +    spice_return_if_fail(base);
> > +
> > +    RedCursorPipeItem *pipe_item = SPICE_CONTAINEROF(base,
> > RedCursorPipeItem, base);
> 
> I don't find this very readable, and imo there is not a huge gain in
> doing this over a cast... I'd just go with a cast + a static
> compile-time assert that RedCursorPipeItem has a 'base' RedPipeItem
> field at offset 0.
> 
> Christophe
> 

So 

static void cursor_pipe_item_free(RedPipeItem *base)
{
   spice_return_if_fail(base);

   verify(SPICE_OFFSETOF(RedCursorPipeItem, base) == 0);
   RedCursorPipeItem *pipe_item = (RedCursorPipeItem *) base;
   ...

(obviously for every cast)

Frediano