[Spice-devel] [PATCH 03/14] Introduce simple RedPipeItem class

[Christophe Fergeau]

On Mon, Apr 11, 2016 at 06:41:19AM -0400, Frediano Ziglio wrote:
> > diff --git a/server/red-pipe-item.c b/server/red-pipe-item.c
> > new file mode 100644
> > index 0000000..a9ebfda
> > --- /dev/null
> > +++ b/server/red-pipe-item.c
> > @@ -0,0 +1,65 @@
> > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > +/*
> > +   Copyright (C) 2015 Red Hat, Inc.
> > +
> > +   This library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   This library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with this library; if not, see
> > <http://www.gnu.org/licenses/>.
> > +*/
> > +#include <config.h>
> > +
> > +#include "red-channel.h"
> > +#include "red-pipe-item.h"
> > +
> > +void pipe_item_init(PipeItem *item, int type)
> > +{
> > +    ring_item_init(&item->link);
> > +    item->type = type;
> > +}
> > +
> > +RedPipeItem *red_pipe_item_ref(gpointer object)
> > +{
> > +    RedPipeItem *item = object;
> > +
> > +    g_return_val_if_fail(item->refcount > 0, NULL);
> > +
> > +    item->refcount++;
> > +
> > +    return item;
> > +}
> > +
> > +void red_pipe_item_unref(gpointer object)
> > +{
> > +    RedPipeItem *item = object;
> > +
> > +    g_return_if_fail(item->refcount > 0);
> > +
> > +    item->refcount--;
> > +    if (item->refcount == 0) {
> > +        if (item->free_func) {
> > +            item->free_func(item);
> > +        } else {
> > +            free(item);
> > +        }
> 
> Why not just
> 
>    item->free_func(item);
> 
> and ...
> 
> > +    }
> > +}
> > +
> > +void red_pipe_item_init(RedPipeItem *item,
> > +                        gint type,
> > +                        GDestroyNotify free_func)
> > +{
> > +    g_return_if_fail(item->refcount == 0);
> > +
> > +    pipe_item_init(&item->parent, type);
> > +    item->refcount = 1;
> > +    item->free_func = free_func;
> 
>    item->free_func = free_func ? free_func : (GDestroyNotify)free;

Yeah, makes things simpler, thanks for the suggestion.

> 
> > +}
> > diff --git a/server/red-pipe-item.h b/server/red-pipe-item.h
> > new file mode 100644
> > index 0000000..7bc5abc
> > --- /dev/null
> > +++ b/server/red-pipe-item.h
> > @@ -0,0 +1,60 @@
> > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> > +/*
> > +   Copyright (C) 2015 Red Hat, Inc.
> > +
> > +   This library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   This library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with this library; if not, see
> > <http://www.gnu.org/licenses/>.
> > +*/
> > +#ifndef __RED_MINI_OBJECT_H__
> > +#define __RED_MINI_OBJECT_H__
> 
> __RED_MINI_OBJECT_H__ ? Beside not being C standard compatible why
> MINI_OBJECT ?

Oh, some remains from a previous experiment, missed it during global
rename.


> 
> > +
> > +#include <glib.h>
> > +
> > +typedef struct PipeItem {
> > +    RingItem link;
> > +    int type;
> > +} PipeItem;
> > +
> > +static inline int pipe_item_is_linked(PipeItem *item)
> > +{
> > +    return ring_item_is_linked(&item->link);
> > +}
> > +
> > +void pipe_item_init(PipeItem *item, int type);
> > +
> > +/* Refcounted pipe item */
> > +/* Future goals are to:
> > + * - merge it with PipeItem
> > + * - stop having RingItem embedded at the beginning of the PipeItem base
> > class
> > + *   but instead have:
> > + *   {
> > + *       int type;
> > + *       int refcount;
> > + *       PipeItem link;
> > + *   }
> > + *   or even drop PipeItem, and use a GList in RedChannel
> 
> Why not just adding this feature to PipeItem now ?

This would definitely makes things easier. Main reason is to avoid the
memory overhead of the additional fields. But I haven't measured how
many PipeItems we typically have at the same time, so it's probably not
a relevant concern.


Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160412/a36dc943/attachment.sig>