[Spice-devel] [spice-server v3 2/3] tests: Port test-qxl-parsing to GTest
Frediano Ziglio
fziglio at redhat.com
Tue Mar 21 15:24:48 UTC 2017
>
> test-qxl-parsing is really a series of several tests. Porting it to
> GTest makes this more obvious. This also has the side-effect of making
> it more friendly to 'make check-valgrind' (which would fail on SIGALRM,
> and on unexpected g_warning()).
> ---
> server/tests/Makefile.am | 1 +
> server/tests/test-qxl-parsing.c | 222
> ++++++++++++++++++++++++++++------------
> 2 files changed, 157 insertions(+), 66 deletions(-)
>
> diff --git a/server/tests/Makefile.am b/server/tests/Makefile.am
> index 3f61faa..fc9e1f2 100644
> --- a/server/tests/Makefile.am
> +++ b/server/tests/Makefile.am
> @@ -124,6 +124,7 @@ test_vdagent_CPPFLAGS = \
> -UGLIB_VERSION_MAX_ALLOWED \
> $(NULL)
> test_codecs_parsing_CPPFLAGS = $(test_vdagent_CPPFLAGS)
> +test_qxl_parsing_CPPFLAGS = $(test_vdagent_CPPFLAGS)
>
> if HAVE_GSTREAMER
> test_gst_SOURCES = test-gst.c \
> diff --git a/server/tests/test-qxl-parsing.c
> b/server/tests/test-qxl-parsing.c
> index dd99150..1d7a71f 100644
> --- a/server/tests/test-qxl-parsing.c
> +++ b/server/tests/test-qxl-parsing.c
> @@ -27,37 +27,25 @@
> #include <stdlib.h>
> #include <stdio.h>
> #include <unistd.h>
> -#include <assert.h>
>
> #include <spice/macros.h>
> #include "memslot.h"
> #include "red-parse-qxl.h"
> +#include "test-glib-compat.h"
>
> -static int exit_code = EXIT_SUCCESS;
> -static const char *test_name = NULL;
> -
> -static void
> -failure(void)
> -{
> - assert(test_name);
> - printf("Test %s FAILED!\n", test_name);
> - exit_code = EXIT_FAILURE;
> -}
> -
> -static void
> -test(const char *desc)
> -{
> - test_name = desc;
> - printf("Starting test %s\n", desc);
> - alarm(5);
> -}
> -
> -static inline QXLPHYSICAL
> +static QXLPHYSICAL
> to_physical(const void *ptr)
> {
> return (uintptr_t) ptr;
> }
>
> +static void *
> +from_physical(QXLPHYSICAL physical)
> +{
> + return (void *) physical;
To avoid warning on 32 bit should be
return (void *) (uintptr_t) physical;
> +}
> +
> +
I would remove an empty line
> static void*
> create_chunk(size_t prefix, uint32_t size, QXLDataChunk* prev, int fill)
> {
> @@ -71,45 +59,77 @@ create_chunk(size_t prefix, uint32_t size, QXLDataChunk*
> prev, int fill)
> return ptr;
> }
>
> -int main(int argc, char **argv)
> +static void init_meminfo(RedMemSlotInfo *mem_info)
> {
> - RedMemSlotInfo mem_info;
> - memslot_info_init(&mem_info, 1 /* groups */, 1 /* slots */, 1, 1, 0);
> - memslot_info_add_slot(&mem_info, 0, 0, 0 /* delta */, 0 /* start */,
> ~0ul /* end */, 0 /* generation */);
> -
> - RedSurfaceCmd cmd;
> - QXLSurfaceCmd qxl;
> -
> - RedCursorCmd red_cursor_cmd;
> - QXLCursorCmd cursor_cmd;
> - QXLCursor *cursor;
> - QXLDataChunk *chunks[2];
> + memslot_info_init(mem_info, 1 /* groups */, 1 /* slots */, 1, 1, 0);
> + memslot_info_add_slot(mem_info, 0, 0, 0 /* delta */, 0 /* start */, ~0ul
> /* end */, 0 /* generation */);
> +}
>
> +static void init_qxl_surface(QXLSurfaceCmd *qxl)
> +{
> void *surface_mem;
>
> - memset(&qxl, 0, sizeof(qxl));
> + memset(qxl, 0, sizeof(*qxl));
>
> - qxl.surface_id = 123;
> + qxl->surface_id = 123;
>
> - /* try to create a surface with no issues, should succeed */
> - test("no issues");
> - qxl.u.surface_create.format = SPICE_SURFACE_FMT_32_xRGB;
> - qxl.u.surface_create.width = 128;
> - qxl.u.surface_create.stride = 512;
> - qxl.u.surface_create.height = 128;
> + qxl->u.surface_create.format = SPICE_SURFACE_FMT_32_xRGB;
> + qxl->u.surface_create.width = 128;
> + qxl->u.surface_create.stride = 512;
> + qxl->u.surface_create.height = 128;
> surface_mem = malloc(0x10000);
> - qxl.u.surface_create.data = to_physical(surface_mem);
> - if (red_get_surface_cmd(&mem_info, 0, &cmd, to_physical(&qxl)))
> - failure();
> + qxl->u.surface_create.data = to_physical(surface_mem);
> +}
> +
> +static void deinit_qxl_surface(QXLSurfaceCmd *qxl)
> +{
> + free(from_physical(qxl->u.surface_create.data));
> +}
> +
> +static void test_no_issues(void)
> +{
> + RedMemSlotInfo mem_info;
> + RedSurfaceCmd cmd;
> + QXLSurfaceCmd qxl;
> +
> + init_meminfo(&mem_info);
> + init_qxl_surface(&qxl);
> +
> + /* try to create a surface with no issues, should succeed */
> + g_assert_false(red_get_surface_cmd(&mem_info, 0, &cmd,
> to_physical(&qxl)));
> +
> + deinit_qxl_surface(&qxl);
> + memslot_info_destroy(&mem_info);
> +}
> +
> +static void test_stride_too_small(void)
> +{
> + RedMemSlotInfo mem_info;
> + RedSurfaceCmd cmd;
> + QXLSurfaceCmd qxl;
> +
> + init_meminfo(&mem_info);
> + init_qxl_surface(&qxl);
>
> /* try to create a surface with a stride too small to fit
> * the entire width.
> * This can be used to cause buffer overflows so refuse it.
> */
> - test("stride too small");
> qxl.u.surface_create.stride = 256;
> - if (!red_get_surface_cmd(&mem_info, 0, &cmd, to_physical(&qxl)))
> - failure();
> + g_assert_true(red_get_surface_cmd(&mem_info, 0, &cmd,
> to_physical(&qxl)));
> +
> + deinit_qxl_surface(&qxl);
> + memslot_info_destroy(&mem_info);
> +}
> +
> +static void test_too_big_image(void)
> +{
> + RedMemSlotInfo mem_info;
> + RedSurfaceCmd cmd;
> + QXLSurfaceCmd qxl;
> +
> + init_meminfo(&mem_info);
> + init_qxl_surface(&qxl);
>
> /* try to create a surface quite large.
> * The sizes (width and height) were chosen so the multiplication
> @@ -118,15 +138,25 @@ int main(int argc, char **argv)
> * overflows. Also the total memory for the card is not enough to
> * hold the surface so surely can't be accepted.
> */
> - test("too big image");
> qxl.u.surface_create.stride = 0x08000004 * 4;
> qxl.u.surface_create.width = 0x08000004;
> qxl.u.surface_create.height = 0x40000020;
> - if (!red_get_surface_cmd(&mem_info, 0, &cmd, to_physical(&qxl)))
> - failure();
> + g_assert_true(red_get_surface_cmd(&mem_info, 0, &cmd,
> to_physical(&qxl)));
> +
> + deinit_qxl_surface(&qxl);
> + memslot_info_destroy(&mem_info);
> +}
> +
> +static void test_cursor_command(void)
> +{
> + RedMemSlotInfo mem_info;
> + RedCursorCmd red_cursor_cmd;
> + QXLCursorCmd cursor_cmd;
> + QXLCursor *cursor;
> +
> + init_meminfo(&mem_info);
>
> /* test base cursor with no problems */
> - test("base cursor command");
> memset(&cursor_cmd, 0, sizeof(cursor_cmd));
> cursor_cmd.type = QXL_CURSOR_SET;
>
> @@ -138,13 +168,25 @@ int main(int argc, char **argv)
>
> cursor_cmd.u.set.shape = to_physical(cursor);
>
> - if (red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd,
> to_physical(&cursor_cmd)))
> - failure();
> + g_assert_false(red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd,
> to_physical(&cursor_cmd)));
> free(red_cursor_cmd.u.set.shape.data);
> free(cursor);
> + memslot_info_destroy(&mem_info);
> +}
> +
> +static void test_circular_empty_chunks(void)
> +{
> + RedMemSlotInfo mem_info;
> + RedCursorCmd red_cursor_cmd;
> + QXLCursorCmd cursor_cmd;
> + QXLCursor *cursor;
> + QXLDataChunk *chunks[2];
> +
> + init_meminfo(&mem_info);
> + g_test_expect_message(G_LOG_DOMAIN, G_LOG_LEVEL_WARNING,
> + "*red_get_data_chunks_ptr: data split in too many
> chunks, avoiding DoS*");
>
> /* a circular list of empty chunks should not be a problems */
> - test("circular empty chunks");
> memset(&cursor_cmd, 0, sizeof(cursor_cmd));
> cursor_cmd.type = QXL_CURSOR_SET;
>
> @@ -162,16 +204,31 @@ int main(int argc, char **argv)
> memset(&red_cursor_cmd, 0xaa, sizeof(red_cursor_cmd));
> if (!red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd,
> to_physical(&cursor_cmd))) {
> /* function does not return errors so there should be no data */
> - assert(red_cursor_cmd.type == QXL_CURSOR_SET);
> - assert(red_cursor_cmd.u.set.position.x == 0);
> - assert(red_cursor_cmd.u.set.position.y == 0);
> - assert(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);
> }
> + g_test_assert_expected_messages();
> +
> free(cursor);
> free(chunks[0]);
> + memslot_info_destroy(&mem_info);
> +}
> +
> +static void test_circular_small_chunks(void)
> +{
> + RedMemSlotInfo mem_info;
> + RedCursorCmd red_cursor_cmd;
> + QXLCursorCmd cursor_cmd;
> + QXLCursor *cursor;
> + QXLDataChunk *chunks[2];
> +
> + init_meminfo(&mem_info);
> + g_test_expect_message(G_LOG_DOMAIN, G_LOG_LEVEL_WARNING,
> + "*red_get_data_chunks_ptr: data split in too many
> chunks, avoiding DoS*");
>
> /* a circular list of small chunks should not be a problems */
> - test("circular small chunks");
> memset(&cursor_cmd, 0, sizeof(cursor_cmd));
> cursor_cmd.type = QXL_CURSOR_SET;
>
> @@ -189,16 +246,49 @@ int main(int argc, char **argv)
> memset(&red_cursor_cmd, 0xaa, sizeof(red_cursor_cmd));
> if (!red_get_cursor_cmd(&mem_info, 0, &red_cursor_cmd,
> to_physical(&cursor_cmd))) {
> /* function does not return errors so there should be no data */
> - assert(red_cursor_cmd.type == QXL_CURSOR_SET);
> - assert(red_cursor_cmd.u.set.position.x == 0);
> - assert(red_cursor_cmd.u.set.position.y == 0);
> - assert(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);
> }
> + g_test_assert_expected_messages();
> +
> free(cursor);
> free(chunks[0]);
> -
> memslot_info_destroy(&mem_info);
> - free(surface_mem);
> +}
> +
> +
> +int main(int argc, char *argv[])
> +{
> + g_test_init(&argc, &argv, NULL);
> +
> + /* try to create a surface with no issues, should succeed */
> + g_test_add_func("/server/qxl-parsing-no-issues", test_no_issues);
> +
> + /* try to create a surface with a stride too small to fit
> + * the entire width.
> + * This can be used to cause buffer overflows so refuse it.
> + */
> + g_test_add_func("/server/qxl-parsing/stride-too-small",
> test_stride_too_small);
> +
> + /* try to create a surface quite large.
> + * The sizes (width and height) were chosen so the multiplication
> + * using 32 bit values gives a very small value.
> + * These kind of values should be refused as they will cause
> + * overflows. Also the total memory for the card is not enough to
> + * hold the surface so surely can't be accepted.
> + */
> + g_test_add_func("/server/qxl-parsing/too-big-image",
> test_too_big_image);
> +
> + /* test base cursor with no problems */
> + g_test_add_func("/server/qxl-parsing/base-cursor-command",
> test_cursor_command);
> +
> + /* a circular list of empty chunks should not be a problems */
> + g_test_add_func("/server/qxl-parsing/circular-empty-chunks",
> test_circular_empty_chunks);
> +
> + /* a circular list of small chunks should not be a problems */
> + g_test_add_func("/server/qxl-parsing/circular-small-chunks",
> test_circular_small_chunks);
>
> - return exit_code;
> + return g_test_run();
> }
Beside that,
Acked-by: Frediano Ziglio <fziglio at redhat.com>
Frediano
More information about the Spice-devel
mailing list