diff --git a/server/Makefile.am b/server/Makefile.am index 2c7479d9..5260051b 100644 --- a/server/Makefile.am +++ b/server/Makefile.am @@ -157,7 +157,7 @@ libserver_la_SOURCES = \ red-pipe-item.h \ red-qxl.cpp \ red-qxl.h \ - red-record-qxl.c \ + red-record-qxl.cpp \ red-record-qxl.h \ red-replay-qxl.cpp \ reds.cpp \ diff --git a/server/cursor-channel.cpp b/server/cursor-channel.cpp index 8bb06134..beb004ea 100644 --- a/server/cursor-channel.cpp +++ b/server/cursor-channel.cpp @@ -26,37 +26,24 @@ #include "reds.h" struct RedCursorPipeItem: public RedPipeItemNum { - explicit RedCursorPipeItem(RedCursorCmd *cmd); - ~RedCursorPipeItem() override; - RedCursorCmd *red_cursor; + explicit RedCursorPipeItem(const red::shared_ptr& cmd); + red::shared_ptr red_cursor; }; -RedCursorPipeItem::RedCursorPipeItem(RedCursorCmd *cmd): - red_cursor(red_cursor_cmd_ref(cmd)) +RedCursorPipeItem::RedCursorPipeItem(const red::shared_ptr& cmd): + red_cursor(cmd) { } -RedCursorPipeItem::~RedCursorPipeItem() -{ - red_cursor_cmd_unref(red_cursor); -} - -static void cursor_channel_set_item(CursorChannel *cursor, RedCursorPipeItem *item) -{ - cursor->item.reset(item); -} - static void cursor_fill(CursorChannelClient *ccc, RedCursorPipeItem *cursor, SpiceCursor *red_cursor, SpiceMarshaller *m) { - RedCursorCmd *cursor_cmd; - if (!cursor) { red_cursor->flags = SPICE_CURSOR_FLAGS_NONE; return; } - cursor_cmd = cursor->red_cursor; + auto cursor_cmd = cursor->red_cursor.get(); *red_cursor = cursor_cmd->u.set.shape; if (red_cursor->header.unique) { @@ -100,11 +87,10 @@ static void red_marshall_cursor(CursorChannelClient *ccc, { CursorChannel *cursor_channel = ccc->get_channel(); RedCursorPipeItem *item = cursor_pipe_item; - RedCursorCmd *cmd; spice_return_if_fail(cursor_channel); - cmd = item->red_cursor; + auto cmd = item->red_cursor.get(); switch (cmd->type) { case QXL_CURSOR_MOVE: { @@ -189,7 +175,7 @@ cursor_channel_new(RedsState *server, int id, return red::make_shared(server, id, core, dispatcher); } -void CursorChannel::process_cmd(RedCursorCmd *cursor_cmd) +void CursorChannel::process_cmd(red::shared_ptr &&cursor_cmd) { bool cursor_show = false; @@ -200,7 +186,7 @@ void CursorChannel::process_cmd(RedCursorCmd *cursor_cmd) switch (cursor_cmd->type) { case QXL_CURSOR_SET: cursor_visible = !!cursor_cmd->u.set.visible; - cursor_channel_set_item(this, cursor_pipe_item.get()); + item = cursor_pipe_item; break; case QXL_CURSOR_MOVE: cursor_show = !cursor_visible; @@ -229,7 +215,7 @@ void CursorChannel::process_cmd(RedCursorCmd *cursor_cmd) void CursorChannel::reset() { - cursor_channel_set_item(this, nullptr); + item.reset(); cursor_visible = true; cursor_position.x = cursor_position.y = 0; cursor_trail_length = cursor_trail_frequency = 0; diff --git a/server/cursor-channel.h b/server/cursor-channel.h index f0f59436..0461a942 100644 --- a/server/cursor-channel.h +++ b/server/cursor-channel.h @@ -38,7 +38,7 @@ struct CursorChannel final: public CommonGraphicsChannel ~CursorChannel(); void reset(); void do_init(); - void process_cmd(RedCursorCmd *cursor_cmd); + void process_cmd(red::shared_ptr &&cursor_cmd); void set_mouse_mode(uint32_t mode); void on_connect(RedClient *client, RedStream *stream, int migration, RedChannelCapabilities *caps) override; diff --git a/server/meson.build b/server/meson.build index fe8d5d63..a8da777f 100644 --- a/server/meson.build +++ b/server/meson.build @@ -136,7 +136,7 @@ spice_server_sources = [ 'red-pipe-item.h', 'red-qxl.cpp', 'red-qxl.h', - 'red-record-qxl.c', + 'red-record-qxl.cpp', 'red-record-qxl.h', 'red-replay-qxl.cpp', 'reds.cpp', diff --git a/server/red-parse-qxl.cpp b/server/red-parse-qxl.cpp index ec4b17e6..198179a3 100644 --- a/server/red-parse-qxl.cpp +++ b/server/red-parse-qxl.cpp @@ -1609,46 +1609,26 @@ static bool red_get_cursor_cmd(QXLInstance *qxl_instance, RedMemSlotInfo *slots, return true; } -RedCursorCmd *red_cursor_cmd_new(QXLInstance *qxl, RedMemSlotInfo *slots, - int group_id, QXLPHYSICAL addr) +red::shared_ptr +red_cursor_cmd_new(QXLInstance *qxl, RedMemSlotInfo *slots, int group_id, QXLPHYSICAL addr) { - RedCursorCmd *cmd; + auto cmd = red::make_shared(); - cmd = g_new0(RedCursorCmd, 1); - - cmd->refs = 1; - - if (!red_get_cursor_cmd(qxl, slots, group_id, cmd, addr)) { - red_cursor_cmd_unref(cmd); - return nullptr; + if (!red_get_cursor_cmd(qxl, slots, group_id, cmd.get(), addr)) { + cmd.reset(); } return cmd; } -static void red_put_cursor_cmd(RedCursorCmd *red) +RedCursorCmd::~RedCursorCmd() { - switch (red->type) { + switch (type) { case QXL_CURSOR_SET: - red_put_cursor(&red->u.set.shape); + red_put_cursor(&u.set.shape); break; } - if (red->qxl) { - red_qxl_release_resource(red->qxl, red->release_info_ext); + if (qxl) { + red_qxl_release_resource(qxl, release_info_ext); } } - -RedCursorCmd *red_cursor_cmd_ref(RedCursorCmd *red) -{ - red->refs++; - return red; -} - -void red_cursor_cmd_unref(RedCursorCmd *red) -{ - if (--red->refs) { - return; - } - red_put_cursor_cmd(red); - g_free(red); -} diff --git a/server/red-parse-qxl.h b/server/red-parse-qxl.h index c7206516..d1aab512 100644 --- a/server/red-parse-qxl.h +++ b/server/red-parse-qxl.h @@ -23,8 +23,9 @@ #include "red-common.h" #include "memslot.h" +#include "utils.hpp" -SPICE_BEGIN_DECLS +#include "push-visibility.h" typedef struct RedDrawable { int refs; @@ -98,10 +99,10 @@ typedef struct RedSurfaceCmd { } u; } RedSurfaceCmd; -typedef struct RedCursorCmd { +struct RedCursorCmd final: public red::simple_ptr_counted { + ~RedCursorCmd(); QXLInstance *qxl; QXLReleaseInfoExt release_info_ext; - int refs; uint8_t type; union { struct { @@ -115,7 +116,7 @@ typedef struct RedCursorCmd { } trail; SpicePoint16 position; } u; -} RedCursorCmd; +}; void red_get_rect_ptr(SpiceRect *red, const QXLRect *qxl); @@ -143,11 +144,9 @@ RedSurfaceCmd *red_surface_cmd_new(QXLInstance *qxl_instance, RedMemSlotInfo *sl 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); -RedCursorCmd *red_cursor_cmd_ref(RedCursorCmd *red); -void red_cursor_cmd_unref(RedCursorCmd *red); +red::shared_ptr +red_cursor_cmd_new(QXLInstance *qxl, RedMemSlotInfo *slots, int group_id, QXLPHYSICAL addr); -SPICE_END_DECLS +#include "pop-visibility.h" #endif /* RED_PARSE_QXL_H_ */ diff --git a/server/red-record-qxl.c b/server/red-record-qxl.cpp similarity index 100% rename from server/red-record-qxl.c rename to server/red-record-qxl.cpp diff --git a/server/red-stream-device.cpp b/server/red-stream-device.cpp index 65eb0d61..92ba0d94 100644 --- a/server/red-stream-device.cpp +++ b/server/red-stream-device.cpp @@ -378,10 +378,10 @@ get_cursor_type_bits(unsigned int cursor_type) } } -static RedCursorCmd * +static red::shared_ptr stream_msg_cursor_set_to_cursor_cmd(const StreamMsgCursorSet *msg, size_t msg_size) { - auto cmd = g_new0(RedCursorCmd, 1); + auto cmd = red::make_shared(); cmd->type = QXL_CURSOR_SET; cmd->u.set.position.x = 0; // TODO cmd->u.set.position.y = 0; // TODO @@ -397,14 +397,12 @@ stream_msg_cursor_set_to_cursor_cmd(const StreamMsgCursorSet *msg, size_t msg_si /* Limit cursor size to prevent DoS */ if (cursor->header.width > STREAM_MSG_CURSOR_SET_MAX_WIDTH || cursor->header.height > STREAM_MSG_CURSOR_SET_MAX_HEIGHT) { - g_free(cmd); - return nullptr; + return red::shared_ptr(); } const unsigned int cursor_bits = get_cursor_type_bits(cursor->header.type); if (cursor_bits == 0) { - g_free(cmd); - return nullptr; + return red::shared_ptr(); } /* Check that enough data has been sent for the cursor. @@ -413,8 +411,7 @@ stream_msg_cursor_set_to_cursor_cmd(const StreamMsgCursorSet *msg, size_t msg_si size_t size_required = cursor->header.width * cursor->header.height; size_required = SPICE_ALIGN(size_required * cursor_bits, 8) / 8U; if (msg_size < sizeof(StreamMsgCursorSet) + size_required) { - g_free(cmd); - return nullptr; + return red::shared_ptr(); } cursor->data_size = size_required; cursor->data = (uint8_t*) g_memdup2(msg->data, size_required); @@ -453,11 +450,11 @@ StreamDevice::handle_msg_cursor_set() } // transform the message to a cursor command and process it - RedCursorCmd *cmd = stream_msg_cursor_set_to_cursor_cmd(&msg->cursor_set, msg_pos); + auto cmd = stream_msg_cursor_set_to_cursor_cmd(&msg->cursor_set, msg_pos); if (!cmd) { return handle_msg_invalid(nullptr); } - cursor_channel->process_cmd(cmd); + cursor_channel->process_cmd(std::move(cmd)); return true; } @@ -478,12 +475,12 @@ StreamDevice::handle_msg_cursor_move() move->x = GINT32_FROM_LE(move->x); move->y = GINT32_FROM_LE(move->y); - auto cmd = g_new0(RedCursorCmd, 1); + auto cmd = red::make_shared(); cmd->type = QXL_CURSOR_MOVE; cmd->u.position.x = move->x; cmd->u.position.y = move->y; - cursor_channel->process_cmd(cmd); + cursor_channel->process_cmd(std::move(cmd)); return true; } diff --git a/server/red-worker.cpp b/server/red-worker.cpp index 2be223b4..9aad023b 100644 --- a/server/red-worker.cpp +++ b/server/red-worker.cpp @@ -90,16 +90,13 @@ struct RedWorker { static gboolean red_process_cursor_cmd(RedWorker *worker, const QXLCommandExt *ext) { - RedCursorCmd *cursor_cmd; - - cursor_cmd = red_cursor_cmd_new(worker->qxl, &worker->mem_slots, - ext->group_id, ext->cmd.data); - if (cursor_cmd == nullptr) { + auto cursor_cmd = red_cursor_cmd_new(worker->qxl, &worker->mem_slots, + ext->group_id, ext->cmd.data); + if (!cursor_cmd) { return FALSE; } - worker->cursor_channel->process_cmd(cursor_cmd); - red_cursor_cmd_unref(cursor_cmd); + worker->cursor_channel->process_cmd(std::move(cursor_cmd)); return TRUE; } diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am index 19058037..5918cb9d 100644 --- a/server/tests/Makefile.am +++ b/server/tests/Makefile.am @@ -83,6 +83,7 @@ endif test_channel_SOURCES = test-channel.cpp test_stream_device_SOURCES = test-stream-device.cpp test_dispatcher_SOURCES = test-dispatcher.cpp +test_qxl_parsing_SOURCES = test-qxl-parsing.cpp if !OS_WIN32 check_PROGRAMS += \ diff --git a/server/tests/meson.build b/server/tests/meson.build index 4f150a0b..1ae7d37c 100644 --- a/server/tests/meson.build +++ b/server/tests/meson.build @@ -44,7 +44,7 @@ tests = [ ['test-stat', true], ['test-agent-msg-filter', true], ['test-loop', true], - ['test-qxl-parsing', true], + ['test-qxl-parsing', true, 'cpp'], ['test-leaks', true], ['test-vdagent', true], ['test-fail-on-null-core-interface', true], diff --git a/server/tests/test-qxl-parsing.c b/server/tests/test-qxl-parsing.cpp similarity index 94% rename from server/tests/test-qxl-parsing.c rename to server/tests/test-qxl-parsing.cpp index ee234b61..510e275b 100644 --- a/server/tests/test-qxl-parsing.c +++ b/server/tests/test-qxl-parsing.cpp @@ -176,7 +176,6 @@ static void test_too_big_image(void) static void test_cursor_command(void) { RedMemSlotInfo mem_info; - RedCursorCmd *red_cursor_cmd; QXLCursorCmd cursor_cmd; QXLCursor *cursor; @@ -194,9 +193,9 @@ static void test_cursor_command(void) cursor_cmd.u.set.shape = to_physical(cursor); - red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0, to_physical(&cursor_cmd)); - g_assert_nonnull(red_cursor_cmd); - red_cursor_cmd_unref(red_cursor_cmd); + auto red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0, to_physical(&cursor_cmd)); + g_assert(red_cursor_cmd); + red_cursor_cmd.reset(); g_free(cursor); memslot_info_destroy(&mem_info); } @@ -204,7 +203,6 @@ static void test_cursor_command(void) static void test_circular_empty_chunks(void) { RedMemSlotInfo mem_info; - RedCursorCmd *red_cursor_cmd; QXLCursorCmd cursor_cmd; QXLCursor *cursor; QXLDataChunk *chunks[2]; @@ -228,14 +226,14 @@ static void test_circular_empty_chunks(void) cursor_cmd.u.set.shape = to_physical(cursor); - red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0, to_physical(&cursor_cmd)); - if (red_cursor_cmd != NULL) { + auto red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0, to_physical(&cursor_cmd)); + if (red_cursor_cmd) { /* 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); - red_cursor_cmd_unref(red_cursor_cmd); + red_cursor_cmd.reset(); } g_test_assert_expected_messages(); @@ -247,7 +245,6 @@ static void test_circular_empty_chunks(void) static void test_circular_small_chunks(void) { RedMemSlotInfo mem_info; - RedCursorCmd *red_cursor_cmd; QXLCursorCmd cursor_cmd; QXLCursor *cursor; QXLDataChunk *chunks[2]; @@ -271,14 +268,14 @@ static void test_circular_small_chunks(void) cursor_cmd.u.set.shape = to_physical(cursor); - red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0, to_physical(&cursor_cmd)); - if (red_cursor_cmd != NULL) { + auto red_cursor_cmd = red_cursor_cmd_new(NULL, &mem_info, 0, to_physical(&cursor_cmd)); + if (red_cursor_cmd) { /* 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); - red_cursor_cmd_unref(red_cursor_cmd); + red_cursor_cmd.reset(); } g_test_assert_expected_messages();