Currently, RedWorker is using stack-allocated variables for RedSurfaceCmd.
Surface commands are rare enough that we can dynamically allocated them
insntead, and make the API in red-parse-qxl.h consistent with how other
QXL commands are handled.
Signed-off-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>
---
server/red-parse-qxl.c | 38 +++++++++++++++++++++++++++++++++++---
server/red-parse-qxl.h | 8 +++++---
server/red-worker.c | 16 ++++++++--------
server/tests/test-qxl-parsing.c | 16 ++++++++++------
4 files changed, 58 insertions(+), 20 deletions(-)
diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index 307856af1..4c4bc4445 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -1418,8 +1418,8 @@ bool red_validate_surface(uint32_t width, uint32_t height,
return true;
}
-bool red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
- RedSurfaceCmd *red, QXLPHYSICAL addr)
+static bool red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
+ RedSurfaceCmd *red, QXLPHYSICAL addr)
{
QXLSurfaceCmd *qxl;
uint64_t size;
@@ -1460,11 +1460,43 @@ bool red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
return true;
}
-void red_put_surface_cmd(RedSurfaceCmd *red)
+static void red_put_surface_cmd(RedSurfaceCmd *red)
{
/* nothing yet */
}
+RedSurfaceCmd *red_surface_cmd_new(QXLInstance *qxl_instance, RedMemSlotInfo *slots,
+ int group_id, QXLPHYSICAL addr)
+{
+ RedSurfaceCmd *cmd;
+
+ cmd = g_new0(RedSurfaceCmd, 1);
+
+ cmd->refs = 1;
+
+ if (!red_get_surface_cmd(slots, group_id, cmd, addr)) {
+ g_free(cmd);
+ return NULL;
+ }
+
+ return cmd;
+}
+
+RedSurfaceCmd *red_surface_cmd_ref(RedSurfaceCmd *cmd)
+{
+ cmd->refs++;
+ return cmd;
+}
+
+void red_surface_cmd_unref(RedSurfaceCmd *cmd)
+{
+ if (--cmd->refs) {
+ return;
+ }
+ red_put_surface_cmd(cmd);
+ g_free(cmd);
+}
+
static bool red_get_cursor(RedMemSlotInfo *slots, int group_id,
SpiceCursor *red, QXLPHYSICAL addr)
{
diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h
index d969b3004..37e4ffb21 100644
--- a/server/red-parse-qxl.h
+++ b/server/red-parse-qxl.h
@@ -86,6 +86,7 @@ typedef struct RedSurfaceCreate {
typedef struct RedSurfaceCmd {
QXLReleaseInfoExt release_info_ext;
+ int refs;
uint32_t surface_id;
uint8_t type;
uint32_t flags;
@@ -132,9 +133,10 @@ void red_message_unref(RedMessage *red);
bool red_validate_surface(uint32_t width, uint32_t height,
int32_t stride, uint32_t format);
-bool red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
- RedSurfaceCmd *red, QXLPHYSICAL addr);
-void red_put_surface_cmd(RedSurfaceCmd *red);
+RedSurfaceCmd *red_surface_cmd_new(QXLInstance *qxl_instance, RedMemSlotInfo *slots,
+ int group_id, QXLPHYSICAL addr);
+RedSurfaceCmd *red_surface_cmd_ref(RedSurfaceCmd *cmd);
+void red_surface_cmd_unref(RedSurfaceCmd *cmd);
RedCursorCmd *red_cursor_cmd_new(QXLInstance *qxl, RedMemSlotInfo *slots,
int group_id, QXLPHYSICAL addr);
diff --git a/server/red-worker.c b/server/red-worker.c
index c29449d76..b0273eee1 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -159,16 +159,16 @@ static int red_process_cursor(RedWorker *worker, int *ring_is_empty)
static gboolean red_process_surface_cmd(RedWorker *worker, QXLCommandExt *ext, gboolean loadvm)
{
- RedSurfaceCmd surface_cmd;
+ RedSurfaceCmd *surface_cmd;
- if (!red_get_surface_cmd(&worker->mem_slots, ext->group_id, &surface_cmd, ext->cmd.data)) {
- return FALSE;
+ surface_cmd = red_surface_cmd_new(worker->qxl, &worker->mem_slots, ext->group_id, ext->cmd.data);
+ if (surface_cmd == NULL) {
+ return false;
}
- display_channel_process_surface_cmd(worker->display_channel, &surface_cmd, loadvm);
- // display_channel_process_surface_cmd() takes ownership of 'release_info_ext',
- // we don't need to release it ourselves
- red_put_surface_cmd(&surface_cmd);
- return TRUE;
+ display_channel_process_surface_cmd(worker->display_channel, surface_cmd, loadvm);
+ red_surface_cmd_unref(surface_cmd);
+
+ return true;
}
static int red_process_display(RedWorker *worker, int *ring_is_empty)
diff --git a/server/tests/test-qxl-parsing.c b/server/tests/test-qxl-parsing.c
index e757bdfac..f244379bc 100644
--- a/server/tests/test-qxl-parsing.c
+++ b/server/tests/test-qxl-parsing.c
@@ -88,14 +88,16 @@ static void deinit_qxl_surface(QXLSurfaceCmd *qxl)
static void test_no_issues(void)
{
RedMemSlotInfo mem_info;
- RedSurfaceCmd cmd;
+ RedSurfaceCmd *cmd;
QXLSurfaceCmd qxl;
init_meminfo(&mem_info);
init_qxl_surface(&qxl);
/* try to create a surface with no issues, should succeed */
- g_assert_true(red_get_surface_cmd(&mem_info, 0, &cmd, to_physical(&qxl)));
+ cmd = red_surface_cmd_new(NULL, &mem_info, 0, to_physical(&qxl));
+ g_assert_nonnull(cmd);
+ red_surface_cmd_unref(cmd);
deinit_qxl_surface(&qxl);
memslot_info_destroy(&mem_info);
@@ -104,7 +106,7 @@ static void test_no_issues(void)
static void test_stride_too_small(void)
{
RedMemSlotInfo mem_info;
- RedSurfaceCmd cmd;
+ RedSurfaceCmd *cmd;
QXLSurfaceCmd qxl;
init_meminfo(&mem_info);
@@ -115,7 +117,8 @@ static void test_stride_too_small(void)
* This can be used to cause buffer overflows so refuse it.
*/
qxl.u.surface_create.stride = 256;
- g_assert_false(red_get_surface_cmd(&mem_info, 0, &cmd, to_physical(&qxl)));
+ cmd = red_surface_cmd_new(NULL, &mem_info, 0, to_physical(&qxl));
+ g_assert_null(cmd);
deinit_qxl_surface(&qxl);
memslot_info_destroy(&mem_info);
@@ -124,7 +127,7 @@ static void test_stride_too_small(void)
static void test_too_big_image(void)
{
RedMemSlotInfo mem_info;
- RedSurfaceCmd cmd;
+ RedSurfaceCmd *cmd;
QXLSurfaceCmd qxl;
init_meminfo(&mem_info);
@@ -140,7 +143,8 @@ static void test_too_big_image(void)
qxl.u.surface_create.stride = 0x08000004 * 4;
qxl.u.surface_create.width = 0x08000004;
qxl.u.surface_create.height = 0x40000020;
- g_assert_false(red_get_surface_cmd(&mem_info, 0, &cmd, to_physical(&qxl)));
+ cmd = red_surface_cmd_new(NULL, &mem_info, 0, to_physical(&qxl));
+ g_assert_null(cmd);
deinit_qxl_surface(&qxl);
memslot_info_destroy(&mem_info);
--
2.14.3
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel