Re: [spice-server v2 6/9] qxl: Add red_cursor_cmd_new and red_cursor_cmd_free helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> Currently, the cursor channel is allocating RedCursorCmd instances itself,
> and then
> calling into red-parse-qxl.h to initialize it, and doing something
> similar when releasing the data. This commit moves this common code to
> red-parse-qxl.[ch]
> 
> Signed-off-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
> ---
>  server/cursor-channel.c         |  6 +-----
>  server/red-parse-qxl.c          | 24 +++++++++++++++++++++---
>  server/red-parse-qxl.h          |  6 +++---
>  server/red-worker.c             | 10 +++++-----
>  server/tests/test-qxl-parsing.c | 37 ++++++++++++++++++++-----------------
>  5 files changed, 50 insertions(+), 33 deletions(-)
> 
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index ada1d62a7..c826627bf 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -69,13 +69,9 @@ static RedCursorPipeItem
> *cursor_pipe_item_new(RedCursorCmd *cmd)
>  
>  static void cursor_pipe_item_free(RedPipeItem *base)
>  {
> -    RedCursorCmd *cursor_cmd;
>      RedCursorPipeItem *pipe_item = SPICE_UPCAST(RedCursorPipeItem, base);
>  
> -    cursor_cmd = pipe_item->red_cursor;
> -    red_put_cursor_cmd(cursor_cmd);
> -    g_free(cursor_cmd);
> -
> +    red_cursor_cmd_free(pipe_item->red_cursor);
>      g_free(pipe_item);
>  }
>  
> diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
> index ccb01d92d..b0c47cfeb 100644
> --- a/server/red-parse-qxl.c
> +++ b/server/red-parse-qxl.c
> @@ -1443,8 +1443,9 @@ static void red_put_cursor(SpiceCursor *red)
>      g_free(red->data);
>  }
>  
> -bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
> -                        RedCursorCmd *red, QXLPHYSICAL addr)
> +static bool red_get_cursor_cmd(QXLInstance *qxl_instance, RedMemSlotInfo
> *slots,
> +                               int group_id, RedCursorCmd *red,
> +                               QXLPHYSICAL addr)
>  {
>      QXLCursorCmd *qxl;
>      int error;
> @@ -1453,6 +1454,7 @@ bool red_get_cursor_cmd(RedMemSlotInfo *slots, int
> group_id,
>      if (error) {
>          return false;
>      }
> +    red->qxl = qxl_instance;
>      red->release_info_ext.info      = &qxl->release_info;
>      red->release_info_ext.group_id  = group_id;
>  
> @@ -1470,10 +1472,25 @@ bool red_get_cursor_cmd(RedMemSlotInfo *slots, int
> group_id,
>          red->u.trail.frequency = qxl->u.trail.frequency;
>          break;
>      }
> +
>      return true;
>  }
>  
> -void red_put_cursor_cmd(RedCursorCmd *red)
> +RedCursorCmd *red_cursor_cmd_new(QXLInstance *qxl, RedMemSlotInfo *slots,
> +                                 int group_id, QXLPHYSICAL addr)

red_cursor_cmd_get ?

> +{
> +    RedCursorCmd *cmd;
> +
> +    cmd = g_new0(RedCursorCmd, 1);
> +    if (!red_get_cursor_cmd(qxl, slots, group_id, cmd, addr)) {
> +        g_free(cmd);
> +        return NULL;
> +    }
> +
> +    return cmd;
> +}
> +
> +void red_cursor_cmd_free(RedCursorCmd *red)
>  {
>      switch (red->type) {
>      case QXL_CURSOR_SET:
> @@ -1483,6 +1500,7 @@ void red_put_cursor_cmd(RedCursorCmd *red)
>      if (red->qxl) {
>          red_qxl_release_resource(red->qxl, red->release_info_ext);
>      }
> +    g_free(red);
>  }
>  
>  void red_drawable_unref(RedDrawable *red_drawable)
> diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
> index 882657e88..231844e96 100644
> --- a/server/red-parse-qxl.h
> +++ b/server/red-parse-qxl.h
> @@ -138,8 +138,8 @@ bool red_get_surface_cmd(RedMemSlotInfo *slots, int
> group_id,
>                           RedSurfaceCmd *red, QXLPHYSICAL addr);
>  void red_put_surface_cmd(RedSurfaceCmd *red);
>  
> -bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id,
> -                        RedCursorCmd *red, QXLPHYSICAL addr);
> -void red_put_cursor_cmd(RedCursorCmd *red);
> +RedCursorCmd *red_cursor_cmd_new(QXLInstance *qxl, RedMemSlotInfo *slots,
> +                                 int group_id, QXLPHYSICAL addr);

OT: all these calls pass group_id and addr from a QXLCommandExt,
maybe we could pass a "const QXLCommandExt*" instead?

> +void red_cursor_cmd_free(RedCursorCmd *cmd);
>  
>  #endif /* RED_PARSE_QXL_H_ */
> diff --git a/server/red-worker.c b/server/red-worker.c
> index ebc6d423d..2dcf2b0ef 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -103,13 +103,14 @@ static gboolean red_process_cursor_cmd(RedWorker
> *worker, const QXLCommandExt *e
>  {
>      RedCursorCmd *cursor_cmd;
>  
> -    cursor_cmd = g_new0(RedCursorCmd, 1);
> -    if (!red_get_cursor_cmd(&worker->mem_slots, ext->group_id, cursor_cmd,
> ext->cmd.data)) {
> -        g_free(cursor_cmd);
> +    cursor_cmd = red_cursor_cmd_new(worker->qxl, &worker->mem_slots,
> +                                    ext->group_id, ext->cmd.data);
> +    if (cursor_cmd == NULL) {
>          return FALSE;
>      }
> -    cursor_cmd->qxl = worker->qxl;
> +
>      cursor_channel_process_cmd(worker->cursor_channel, cursor_cmd);
> +
>      return TRUE;
>  }
>  
> @@ -965,7 +966,6 @@ static bool loadvm_command(RedWorker *worker,
> QXLCommandExt *ext)
>      switch (ext->cmd.type) {
>      case QXL_CMD_CURSOR:
>          return red_process_cursor_cmd(worker, ext);
> -

spurious

>      case QXL_CMD_SURFACE:
>          return red_process_surface_cmd(worker, ext, TRUE);
>  
> diff --git a/server/tests/test-qxl-parsing.c
> b/server/tests/test-qxl-parsing.c
> index 47139a48c..f2b12ad07 100644
> --- a/server/tests/test-qxl-parsing.c
> +++ b/server/tests/test-qxl-parsing.c
> @@ -149,7 +149,7 @@ static void test_too_big_image(void)
>  static void test_cursor_command(void)
>  {
>      RedMemSlotInfo mem_info;
> -    RedCursorCmd red_cursor_cmd;
> +    RedCursorCmd *red_cursor_cmd;
>      QXLCursorCmd cursor_cmd;
>      QXLCursor *cursor;
>  
> @@ -167,8 +167,9 @@ static void test_cursor_command(void)
>  
>      cursor_cmd.u.set.shape = to_physical(cursor);
>  
> -    g_assert_true(red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd,
> to_physical(&cursor_cmd)));
> -    g_free(red_cursor_cmd.u.set.shape.data);
> +    red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0,
> to_physical(&cursor_cmd));
> +    g_assert_true(red_cursor_cmd != NULL);
> +    red_cursor_cmd_free(red_cursor_cmd);
>      g_free(cursor);
>      memslot_info_destroy(&mem_info);
>  }
> @@ -176,7 +177,7 @@ static void test_cursor_command(void)
>  static void test_circular_empty_chunks(void)
>  {
>      RedMemSlotInfo mem_info;
> -    RedCursorCmd red_cursor_cmd;
> +    RedCursorCmd *red_cursor_cmd;
>      QXLCursorCmd cursor_cmd;
>      QXLCursor *cursor;
>      QXLDataChunk *chunks[2];
> @@ -200,13 +201,14 @@ static void test_circular_empty_chunks(void)
>  
>      cursor_cmd.u.set.shape = to_physical(cursor);
>  
> -    memset(&red_cursor_cmd, 0xaa, sizeof(red_cursor_cmd));
> -    if (red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd,
> to_physical(&cursor_cmd))) {
> +    red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0,
> to_physical(&cursor_cmd));
> +    if (red_cursor_cmd != NULL) {
>          /* function does not return errors so there should be no data */
> -        g_assert_cmpuint(red_cursor_cmd.type, ==, QXL_CURSOR_SET);
> -        g_assert_cmpuint(red_cursor_cmd.u.set.position.x, ==, 0);
> -        g_assert_cmpuint(red_cursor_cmd.u.set.position.y, ==, 0);
> -        g_assert_cmpuint(red_cursor_cmd.u.set.shape.data_size, ==, 0);
> +        g_assert_cmpuint(red_cursor_cmd->type, ==, QXL_CURSOR_SET);
> +        g_assert_cmpuint(red_cursor_cmd->u.set.position.x, ==, 0);
> +        g_assert_cmpuint(red_cursor_cmd->u.set.position.y, ==, 0);
> +        g_assert_cmpuint(red_cursor_cmd->u.set.shape.data_size, ==, 0);
> +        red_cursor_cmd_free(red_cursor_cmd);
>      }
>      g_test_assert_expected_messages();
>  
> @@ -218,7 +220,7 @@ static void test_circular_empty_chunks(void)
>  static void test_circular_small_chunks(void)
>  {
>      RedMemSlotInfo mem_info;
> -    RedCursorCmd red_cursor_cmd;
> +    RedCursorCmd *red_cursor_cmd;
>      QXLCursorCmd cursor_cmd;
>      QXLCursor *cursor;
>      QXLDataChunk *chunks[2];
> @@ -242,13 +244,14 @@ static void test_circular_small_chunks(void)
>  
>      cursor_cmd.u.set.shape = to_physical(cursor);
>  
> -    memset(&red_cursor_cmd, 0xaa, sizeof(red_cursor_cmd));
> -    if (red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd,
> to_physical(&cursor_cmd))) {
> +    red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0,
> to_physical(&cursor_cmd));
> +    if (red_cursor_cmd != NULL) {
>          /* function does not return errors so there should be no data */
> -        g_assert_cmpuint(red_cursor_cmd.type, ==, QXL_CURSOR_SET);
> -        g_assert_cmpuint(red_cursor_cmd.u.set.position.x, ==, 0);
> -        g_assert_cmpuint(red_cursor_cmd.u.set.position.y, ==, 0);
> -        g_assert_cmpuint(red_cursor_cmd.u.set.shape.data_size, ==, 0);
> +        g_assert_cmpuint(red_cursor_cmd->type, ==, QXL_CURSOR_SET);
> +        g_assert_cmpuint(red_cursor_cmd->u.set.position.x, ==, 0);
> +        g_assert_cmpuint(red_cursor_cmd->u.set.position.y, ==, 0);
> +        g_assert_cmpuint(red_cursor_cmd->u.set.shape.data_size, ==, 0);
> +        red_cursor_cmd_free(red_cursor_cmd);
>      }
>      g_test_assert_expected_messages();
>  

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]