[Spice-devel] [spice-server v2 6/9] qxl: Add red_cursor_cmd_new and red_cursor_cmd_free helpers
Frediano Ziglio
fziglio at redhat.com
Tue Apr 17 19:11:04 UTC 2018
>
> 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 at redhat.com>
> ---
> 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
More information about the Spice-devel
mailing list