[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