[Spice-devel] [PATCH spice-server 02/12] Be consistent with opaque type

[Frediano Ziglio]

> 
> On Tue, Oct 18, 2016 at 10:09:47AM +0100, Frediano Ziglio wrote:
> > vdi_port_read_buf_release is registered passing data as
> > RedVDIReadBuf*, not RedPipeItem*. Cast opaque to proper
> > pointer type to avoid the assumption that first field of
> > RedVDIReadBuf is a RedPipeItem.
> 
> My initial objection still stands here, RedVDIReadBuf has a 'base'
> field as its first member, and it's very much meant to be a RedPipeItem
> as a way to get refcounting for free. So I much prefer the current
> version where we have a cast from child to base type, rather than
> apparently unref'ing something contained in the read_buf object.
> 
> Christophe
> 

I replied quite a month ago, see https://lists.freedesktop.org/archives/spice-devel/2016-September/032360.html.
I took the not reply as a neutral position from you and I posted again the patch twice and
at the end was acked so I accepted the ack and merged it.
I think that my reasoning is more focuses on the "get refcounting for free" not being
a great design so willing to change and I prefer a compile error in the future
instead on the "base type".

Frediano

> 
> 
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/reds.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/server/reds.c b/server/reds.c
> > index 79f9c9e..a71029f 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -745,7 +745,8 @@ static void reds_agent_remove(RedsState *reds)
> >  
> >  static void vdi_port_read_buf_release(uint8_t *data, void *opaque)
> >  {
> > -    red_pipe_item_unref((RedPipeItem *)opaque);
> > +    RedVDIReadBuf *read_buf = (RedVDIReadBuf *)opaque;
> > +    red_pipe_item_unref(&read_buf->base);
> >  }
> >  
> >  /*
> > --
> > 2.7.4
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
>