On Mon, Sep 04, 2017 at 12:02:10PM +0100, Frediano Ziglio wrote:
> RedPipeItem already implements reference counting so
> this avoid duplicating code to handle a object with reference
> counting that points to another object with reference counting
> that holds a RedCursorCmd.
Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
Looks good to me apart from one minor comment below
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> server/cursor-channel.c | 107 +++++++++++++++---------------------------------
> 1 file changed, 33 insertions(+), 74 deletions(-)
>
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index 2441e1ed..2efa6ef4 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -28,17 +28,17 @@
> #include "reds.h"
> #include "red-qxl.h"
>
> -typedef struct CursorItem {
> +typedef struct RedCursorPipeItem {
> + RedPipeItem base;
> QXLInstance *qxl;
> - int refs;
> RedCursorCmd *red_cursor;
> -} CursorItem;
> +} RedCursorPipeItem;
>
> struct CursorChannel
> {
> CommonGraphicsChannel parent;
>
> - CursorItem *item;
> + RedCursorPipeItem *item;
> bool cursor_visible;
> SpicePoint16 cursor_position;
> uint16_t cursor_trail_length;
> @@ -51,83 +51,52 @@ struct CursorChannelClass
> CommonGraphicsChannelClass parent_class;
> };
>
> -typedef struct RedCursorPipeItem {
> - RedPipeItem base;
> - CursorItem *cursor_item;
> -} RedCursorPipeItem;
> -
> G_DEFINE_TYPE(CursorChannel, cursor_channel, TYPE_COMMON_GRAPHICS_CHANNEL)
>
> static void cursor_pipe_item_free(RedPipeItem *pipe_item);
>
> -static CursorItem *cursor_item_new(QXLInstance *qxl, RedCursorCmd *cmd)
> +static RedCursorPipeItem *cursor_pipe_item_new(QXLInstance *qxl, RedCursorCmd *cmd)
> {
> - CursorItem *cursor_item;
> + RedCursorPipeItem *item = spice_malloc0(sizeof(RedCursorPipeItem));
>
> spice_return_val_if_fail(cmd != NULL, NULL);
>
> - cursor_item = g_new0(CursorItem, 1);
> - cursor_item->qxl = qxl;
> - cursor_item->refs = 1;
> - cursor_item->red_cursor = cmd;
> + red_pipe_item_init_full(&item->base, RED_PIPE_ITEM_TYPE_CURSOR,
> + cursor_pipe_item_free);
> + item->qxl = qxl;
> + item->red_cursor = cmd;
>
> - return cursor_item;
> + return item;
> }
>
> -static CursorItem *cursor_item_ref(CursorItem *item)
> +static RedCursorPipeItem *cursor_pipe_item_ref(RedCursorPipeItem *item)
> {
> - spice_return_val_if_fail(item != NULL, NULL);
> - spice_return_val_if_fail(item->refs > 0, NULL);
> -
> - item->refs++;
> -
> + red_pipe_item_ref(&item->base);
> return item;
> }
I would not keep this cursor_pipe_item_ref() wrapper
Christophe
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel