[Spice-devel] [spice-server v2 4/9] qxl: Make red_{get, put}_drawable static

[Frediano Ziglio]

> 
> Rather than needing to call red_drawable_new() and then initialize it
> with red_get_drawable(), we can improve slightly red_drawable new so
> that red_drawable_{new,ref,unref} is all which is used by code out of
> red-parse-qxl.c.
> 
> Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> ---
>  server/red-parse-qxl.c | 17 ++++++++++++-----
>  server/red-parse-qxl.h |  9 ++++-----
>  server/red-worker.c    | 11 ++++++-----
>  3 files changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
> index 245235082..cc6a8b51d 100644
> --- a/server/red-parse-qxl.c
> +++ b/server/red-parse-qxl.c
> @@ -1176,8 +1176,8 @@ static bool red_get_compat_drawable(RedMemSlotInfo
> *slots, int group_id,
>      return true;
>  }
>  
> -bool red_get_drawable(RedMemSlotInfo *slots, int group_id,
> -                      RedDrawable *red, QXLPHYSICAL addr, uint32_t flags)
> +static bool red_get_drawable(RedMemSlotInfo *slots, int group_id,
> +                             RedDrawable *red, QXLPHYSICAL addr, uint32_t
> flags)
>  {
>      bool ret;
>  
> @@ -1189,7 +1189,7 @@ bool red_get_drawable(RedMemSlotInfo *slots, int
> group_id,
>      return ret;
>  }
>  
> -void red_put_drawable(RedDrawable *red)
> +static void red_put_drawable(RedDrawable *red)
>  {
>      red_put_clip(&red->clip);
>      if (red->self_bitmap_image) {
> @@ -1492,12 +1492,19 @@ void red_drawable_unref(RedDrawable *red_drawable)
>      g_free(red_drawable);
>  }
>  
> -RedDrawable *red_drawable_new(QXLInstance *qxl)
> +RedDrawable *red_drawable_new(QXLInstance *qxl, RedMemSlotInfo *slots,
> +                              int group_id, QXLPHYSICAL addr,
> +                              uint32_t flags)
>  {
> -    RedDrawable * red = g_new0(RedDrawable, 1);
> +    RedDrawable *red = g_new0(RedDrawable, 1);
>  
>      red->refs = 1;
>      red->qxl = qxl;
>  
> +    if (!red_get_drawable(slots, group_id, red, addr, flags)) {
> +       red_drawable_unref(red);
> +       return NULL;
> +    }
> +
>      return red;
>  }

red_drawable_new is confusing if is also reading from QXL memory,
red_drawable_get IMHO would be better

> diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
> index 0b507b93a..882657e88 100644
> --- a/server/red-parse-qxl.h
> +++ b/server/red-parse-qxl.h
> @@ -64,8 +64,6 @@ static inline RedDrawable *red_drawable_ref(RedDrawable
> *drawable)
>      drawable->refs++;
>      return drawable;
>  }
> -RedDrawable *red_drawable_new(QXLInstance *qxl);
> -void red_drawable_unref(RedDrawable *red_drawable);
>  
>  void red_drawable_unref(RedDrawable *red_drawable);
>  
> @@ -120,9 +118,10 @@ typedef struct RedCursorCmd {
>  
>  void red_get_rect_ptr(SpiceRect *red, const QXLRect *qxl);
>  
> -bool red_get_drawable(RedMemSlotInfo *slots, int group_id,
> -                      RedDrawable *red, QXLPHYSICAL addr, uint32_t flags);
> -void red_put_drawable(RedDrawable *red);
> +RedDrawable *red_drawable_new(QXLInstance *qxl, RedMemSlotInfo *slots,
> +                              int group_id, QXLPHYSICAL addr,
> +                              uint32_t flags);
> +void red_drawable_unref(RedDrawable *red_drawable);
>  
>  bool red_get_update_cmd(RedMemSlotInfo *slots, int group_id,
>                          RedUpdateCmd *red, QXLPHYSICAL addr);
> diff --git a/server/red-worker.c b/server/red-worker.c
> index 8f806e8e3..ebc6d423d 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -205,15 +205,16 @@ static int red_process_display(RedWorker *worker, int
> *ring_is_empty)
>          worker->display_poll_tries = 0;
>          switch (ext_cmd.cmd.type) {
>          case QXL_CMD_DRAW: {
> -            RedDrawable *red_drawable = red_drawable_new(worker->qxl); //
> returns with 1 ref
> +            RedDrawable *red_drawable;
> +            red_drawable = red_drawable_new(worker->qxl, &worker->mem_slots,
> +                                            ext_cmd.group_id,
> ext_cmd.cmd.data,
> +                                            ext_cmd.flags); // returns with
> 1 ref
>  
> -            if (red_get_drawable(&worker->mem_slots, ext_cmd.group_id,
> -                                 red_drawable, ext_cmd.cmd.data,
> ext_cmd.flags)) {
> +            if (red_drawable != NULL) {
>                  display_channel_process_draw(worker->display_channel,
>                  red_drawable,
>                                               worker->process_display_generation);
> +                red_drawable_unref(red_drawable);
>              }
> -            // release the red_drawable

this comment was really... unhelpful :-)

> -            red_drawable_unref(red_drawable);
>              break;
>          }
>          case QXL_CMD_UPDATE: {

Frediano