[Spice-commits] 4 commits - server/red-replay-qxl.c

Frediano Ziglio fziglio at kemper.freedesktop.org
Wed Sep 21 16:58:45 UTC 2016


 server/red-replay-qxl.c |  204 +++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 166 insertions(+), 38 deletions(-)

New commits:
commit 0e5464f15229e24c3c2f65201cbb9947c125c402
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Thu Sep 15 18:24:17 2016 +0100

    replay: Move error check
    
    Do the check after replay_fscanf to make sure everything
    is fine before calling red_replay_compat_drawable or
    red_replay_native_drawable.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index a52a069..6e2bbb6 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -1066,10 +1066,10 @@ static QXLCompatDrawable *red_replay_compat_drawable(SpiceReplay *replay, uint32
 
 static QXLPHYSICAL red_replay_drawable(SpiceReplay *replay, uint32_t flags)
 {
+    replay_fscanf(replay, "drawable\n");
     if (replay->error) {
         return 0;
     }
-    replay_fscanf(replay, "drawable\n");
     if (flags & QXL_COMMAND_FLAG_COMPAT) {
         return QXLPHYSICAL_FROM_PTR(red_replay_compat_drawable(replay, flags));
     } else {
commit 6c5ea7311aad6126b499c64bbf589ff9236b35d6
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Thu Sep 15 18:23:14 2016 +0100

    replay: Detect errors from red_replay_data_chunks
    
    Change the return to ssize_t to be able to distinguish from
    empty buffer to error.
    Check result returned and avoid continuing potentially
    deferencing NULL pointers.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index b6ad747..a52a069 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -282,8 +282,8 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
     return replay_fscanf(replay, "\n");
 }
 
-static size_t red_replay_data_chunks(SpiceReplay *replay, const char *prefix,
-                                     uint8_t **mem, size_t base_size)
+static ssize_t red_replay_data_chunks(SpiceReplay *replay, const char *prefix,
+                                      uint8_t **mem, size_t base_size)
 {
     size_t data_size;
     int count_chunks;
@@ -291,12 +291,15 @@ static size_t red_replay_data_chunks(SpiceReplay *replay, const char *prefix,
     QXLDataChunk *cur, *next;
 
     replay_fscanf(replay, "data_chunks %d %zu\n", &count_chunks, &data_size);
+    if (replay->error) {
+        return -1;
+    }
     if (base_size == 0) {
         base_size = sizeof(QXLDataChunk);
     }
 
     if (read_binary(replay, prefix, &next_data_size, mem, base_size) == REPLAY_ERROR) {
-        return 0;
+        return -1;
     }
     cur = (QXLDataChunk*)(*mem + base_size - sizeof(QXLDataChunk));
     cur->data_size = next_data_size;
@@ -305,7 +308,7 @@ static size_t red_replay_data_chunks(SpiceReplay *replay, const char *prefix,
     while (count_chunks-- > 0) {
         if (read_binary(replay, prefix, &next_data_size, (uint8_t**)&cur->next_chunk,
             sizeof(QXLDataChunk)) == REPLAY_ERROR) {
-            return 0;
+            return -1;
         }
         data_size += next_data_size;
         next = QXLPHYSICAL_TO_PTR(cur->next_chunk);
@@ -358,9 +361,12 @@ static void red_replay_rect_ptr(SpiceReplay *replay, const char *prefix, QXLRect
 static QXLPath *red_replay_path(SpiceReplay *replay)
 {
     QXLPath *qxl = NULL;
-    size_t data_size;
+    ssize_t data_size;
 
     data_size = red_replay_data_chunks(replay, "path", (uint8_t**)&qxl, sizeof(QXLPath));
+    if (data_size < 0) {
+        return NULL;
+    }
     qxl->data_size = data_size;
     return qxl;
 }
@@ -381,7 +387,9 @@ static QXLClipRects *red_replay_clip_rects(SpiceReplay *replay)
     if (replay->error) {
         return NULL;
     }
-    red_replay_data_chunks(replay, "clip_rects", (uint8_t**)&qxl, sizeof(QXLClipRects));
+    if (red_replay_data_chunks(replay, "clip_rects", (uint8_t**)&qxl, sizeof(QXLClipRects)) < 0) {
+        return NULL;
+    }
     qxl->num_rects = num_rects;
     return qxl;
 }
@@ -402,7 +410,8 @@ static uint8_t *red_replay_image_data_flat(SpiceReplay *replay, size_t *size)
 static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
 {
     QXLImage* qxl = NULL;
-    size_t bitmap_size, size;
+    size_t bitmap_size;
+    ssize_t size;
     uint8_t qxl_flags;
     int temp;
     int has_palette;
@@ -717,13 +726,16 @@ static QXLString *red_replay_string(SpiceReplay *replay)
     uint32_t data_size;
     uint16_t length;
     uint16_t flags;
-    size_t chunk_size;
+    ssize_t chunk_size;
     QXLString *qxl = NULL;
 
     replay_fscanf(replay, "data_size %d\n", &data_size);
     replay_fscanf(replay, "length %d\n", &temp); length = temp;
     replay_fscanf(replay, "flags %d\n", &temp); flags = temp;
     chunk_size = red_replay_data_chunks(replay, "string", (uint8_t**)&qxl, sizeof(QXLString));
+    if (chunk_size <  0) {
+        return NULL;
+    }
     qxl->data_size = data_size;
     qxl->length = length;
     qxl->flags = flags;
@@ -1146,6 +1158,7 @@ static QXLCursor *red_replay_cursor(SpiceReplay *replay)
 {
     int temp;
     QXLCursor cursor, *qxl = NULL;
+    ssize_t data_size;
 
     replay_fscanf(replay, "header.unique %"SCNu64"\n", &cursor.header.unique);
     replay_fscanf(replay, "header.type %d\n", &temp);
@@ -1163,10 +1176,12 @@ static QXLCursor *red_replay_cursor(SpiceReplay *replay)
     if (replay->error) {
         return NULL;
     }
-    cursor.data_size = temp;
-    cursor.data_size = red_replay_data_chunks(replay, "cursor", (uint8_t**)&qxl, sizeof(QXLCursor));
+    data_size = red_replay_data_chunks(replay, "cursor", (uint8_t**)&qxl, sizeof(QXLCursor));
+    if (data_size < 0) {
+        return NULL;
+    }
     qxl->header = cursor.header;
-    qxl->data_size = cursor.data_size;
+    qxl->data_size = data_size;
     return qxl;
 }
 
commit 1c8a01e58dd5c78432f0a59a9cf3156d510c13cc
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Thu Sep 15 18:19:42 2016 +0100

    replay: Handle errors reading strings from record file
    
    To check fscanf read all needed information a dummy "%n" is appended
    to any string and the value stored there is tested. This as scanf family
    could return a valid value but not entirely process the string so
    adding a "%n" and checking this was processed make sure all expected
    string is found.
    The code to check for a specific string is now a bit more complicated
    as replay_fscanf use a macro which append a constant string.
    The "error" field is used to mark any error happened, so in most cases
    there is no explicit check beside when this could cause a problem
    (for instance results of replay_fscanf used which would result in
    uninitialised variable usage).
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index 8229d5e..b6ad747 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -49,6 +49,7 @@ struct SpiceReplay {
     GArray *id_map_inv; // replay id -> record id
     GArray *id_free; // free list
     int nsurfaces;
+    int end_pos;
 
     GList *allocated;
 
@@ -69,11 +70,13 @@ static int replay_fread(SpiceReplay *replay, uint8_t *buf, size_t size)
 }
 
 __attribute__((format(scanf, 2, 3)))
-static replay_t replay_fscanf(SpiceReplay *replay, const char *fmt, ...)
+static replay_t replay_fscanf_check(SpiceReplay *replay, const char *fmt, ...)
 {
     va_list ap;
     int ret;
 
+    replay->end_pos = -1;
+
     if (replay->error) {
         return REPLAY_ERROR;
     }
@@ -84,11 +87,13 @@ static replay_t replay_fscanf(SpiceReplay *replay, const char *fmt, ...)
     va_start(ap, fmt);
     ret = vfscanf(replay->fd, fmt, ap);
     va_end(ap);
-    if (ret == EOF) {
+    if (ret == EOF || replay->end_pos < 0) {
         replay->error = TRUE;
     }
     return replay->error ? REPLAY_ERROR : REPLAY_OK;
 }
+#define replay_fscanf(r, fmt, ...) \
+    replay_fscanf_check(r, fmt "%n", ## __VA_ARGS__, &r->end_pos)
 
 static inline void *replay_malloc(SpiceReplay *replay, size_t size)
 {
@@ -216,9 +221,11 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
     uint8_t *zlib_buffer;
     z_stream strm;
 
-    snprintf(template, sizeof(template), "binary %%d %s %%ld:", prefix);
-    if (replay_fscanf(replay, template, &with_zlib, size) == REPLAY_ERROR)
+    snprintf(template, sizeof(template), "binary %%d %s %%ld:%%n", prefix);
+    replay_fscanf_check(replay, template, &with_zlib, size, &replay->end_pos);
+    if (replay->error) {
         return REPLAY_ERROR;
+    }
 
     if (*buf == NULL) {
         *buf = replay_malloc(replay, *size + base_size);
@@ -230,13 +237,17 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
         hexdump(*buf + base_size, *size);
     }
 #else
-    spice_return_val_if_fail(with_zlib != -1, REPLAY_ERROR);
     if (with_zlib) {
         int ret;
 
         replay_fscanf(replay, "%d:", &zlib_size);
+        if (replay->error) {
+            return REPLAY_ERROR;
+        }
         zlib_buffer = replay_malloc(replay, zlib_size);
-        replay_fread(replay, zlib_buffer, zlib_size);
+        if (replay_fread(replay, zlib_buffer, zlib_size) != zlib_size) {
+            return REPLAY_ERROR;
+        }
         strm.zalloc = Z_NULL;
         strm.zfree = Z_NULL;
         strm.opaque = Z_NULL;
@@ -268,8 +279,7 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
         replay_fread(replay, *buf + base_size, *size);
     }
 #endif
-    replay_fscanf(replay, "\n");
-    return REPLAY_OK;
+    return replay_fscanf(replay, "\n");
 }
 
 static size_t red_replay_data_chunks(SpiceReplay *replay, const char *prefix,
@@ -340,8 +350,9 @@ static void red_replay_rect_ptr(SpiceReplay *replay, const char *prefix, QXLRect
 {
     char template[1024];
 
-    snprintf(template, sizeof(template), "rect %s %%d %%d %%d %%d\n", prefix);
-    replay_fscanf(replay, template, &qxl->top, &qxl->left, &qxl->bottom, &qxl->right);
+    snprintf(template, sizeof(template), "rect %s %%d %%d %%d %%d\n%%n", prefix);
+    replay_fscanf_check(replay, template, &qxl->top, &qxl->left, &qxl->bottom, &qxl->right,
+                        &replay->end_pos);
 }
 
 static QXLPath *red_replay_path(SpiceReplay *replay)
@@ -367,6 +378,9 @@ static QXLClipRects *red_replay_clip_rects(SpiceReplay *replay)
     int num_rects;
 
     replay_fscanf(replay, "num_rects %d\n", &num_rects);
+    if (replay->error) {
+        return NULL;
+    }
     red_replay_data_chunks(replay, "clip_rects", (uint8_t**)&qxl, sizeof(QXLClipRects));
     qxl->num_rects = num_rects;
     return qxl;
@@ -395,6 +409,9 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
     int has_image;
 
     replay_fscanf(replay, "image %d\n", &has_image);
+    if (replay->error) {
+        return NULL;
+    }
     if (!has_image) {
         return NULL;
     }
@@ -405,6 +422,9 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
     replay_fscanf(replay, "descriptor.flags %d\n", &temp); qxl->descriptor.flags = temp;
     replay_fscanf(replay, "descriptor.width %d\n", &qxl->descriptor.width);
     replay_fscanf(replay, "descriptor.height %d\n", &qxl->descriptor.height);
+    if (replay->error) {
+        return NULL;
+    }
 
     switch (qxl->descriptor.type) {
     case SPICE_IMAGE_TYPE_BITMAP:
@@ -420,6 +440,9 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
             int i, num_ents;
 
             replay_fscanf(replay, "qp.num_ents %d\n", &num_ents);
+            if (replay->error) {
+                return NULL;
+            }
             qp = replay_malloc(replay, sizeof(QXLPalette) + num_ents * sizeof(qp->ents[0]));
             qp->num_ents = num_ents;
             qxl->bitmap.palette = QXLPHYSICAL_FROM_PTR(qp);
@@ -444,12 +467,18 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
         break;
     case SPICE_IMAGE_TYPE_SURFACE:
         replay_fscanf(replay, "surface_image.surface_id %d\n", &qxl->surface_image.surface_id);
+        if (replay->error) {
+            return NULL;
+        }
         qxl->surface_image.surface_id = replay_id_get(replay, qxl->surface_image.surface_id);
         break;
     case SPICE_IMAGE_TYPE_QUIC:
         // TODO - make this much nicer (precompute size and allocs, store them during
         // record, then reread into them. and use MPEG-4).
         replay_fscanf(replay, "quic.data_size %d\n", &qxl->quic.data_size);
+        if (replay->error) {
+            return NULL;
+        }
         qxl = realloc(qxl, sizeof(QXLImageDescriptor) + sizeof(QXLQUICData) +
                       qxl->quic.data_size);
         size = red_replay_data_chunks(replay, "quic.data", (uint8_t**)&qxl->quic.data, 0);
@@ -491,6 +520,10 @@ static void red_replay_image_free(SpiceReplay *replay, QXLPHYSICAL p, uint32_t f
 static void red_replay_brush_ptr(SpiceReplay *replay, QXLBrush *qxl, uint32_t flags)
 {
     replay_fscanf(replay, "type %d\n", &qxl->type);
+    if (replay->error) {
+        return;
+    }
+
     switch (qxl->type) {
     case SPICE_BRUSH_TYPE_SOLID:
         replay_fscanf(replay, "u.color %d\n", &qxl->u.color);
@@ -655,6 +688,9 @@ static void red_replay_stroke_ptr(SpiceReplay *replay, QXLStroke *qxl, uint32_t
 
     qxl->path = QXLPHYSICAL_FROM_PTR(red_replay_path(replay));
     replay_fscanf(replay, "attr.flags %d\n", &temp); qxl->attr.flags = temp;
+    if (replay->error) {
+        return;
+    }
     if (qxl->attr.flags & SPICE_LINE_FLAGS_STYLED) {
         size_t size;
 
@@ -752,6 +788,9 @@ static void red_replay_invers_free(SpiceReplay *replay, QXLInvers *qxl, uint32_t
 static void red_replay_clip_ptr(SpiceReplay *replay, QXLClip *qxl)
 {
     replay_fscanf(replay, "type %d\n", &qxl->type);
+    if (replay->error) {
+        return;
+    }
     switch (qxl->type) {
     case SPICE_CLIP_TYPE_RECTS:
         qxl->data = QXLPHYSICAL_FROM_PTR(red_replay_clip_rects(replay));
@@ -781,7 +820,7 @@ static uint8_t *red_replay_transform(SpiceReplay *replay)
 
 static void red_replay_composite_ptr(SpiceReplay *replay, QXLComposite *qxl, uint32_t flags)
 {
-    int enabled;
+    int enabled = 0;
 
     replay_fscanf(replay, "flags %d\n", &qxl->flags);
     qxl->src = QXLPHYSICAL_FROM_PTR(red_replay_image(replay, flags));
@@ -821,15 +860,24 @@ static QXLDrawable *red_replay_native_drawable(SpiceReplay *replay, uint32_t fla
     replay_fscanf(replay, "self_bitmap %d\n", &temp); qxl->self_bitmap = temp;
     red_replay_rect_ptr(replay, "self_bitmap_area", &qxl->self_bitmap_area);
     replay_fscanf(replay, "surface_id %d\n", &qxl->surface_id);
+    if (replay->error) {
+        return NULL;
+    }
     qxl->surface_id = replay_id_get(replay, qxl->surface_id);
 
     for (i = 0; i < 3; i++) {
         replay_fscanf(replay, "surfaces_dest %d\n", &qxl->surfaces_dest[i]);
+        if (replay->error) {
+            return NULL;
+        }
         qxl->surfaces_dest[i] = replay_id_get(replay, qxl->surfaces_dest[i]);
         red_replay_rect_ptr(replay, "surfaces_rects", &qxl->surfaces_rects[i]);
     }
 
     replay_fscanf(replay, "type %d\n", &temp); qxl->type = temp;
+    if (replay->error) {
+        return NULL;
+    }
     switch (qxl->type) {
     case QXL_DRAW_ALPHA_BLEND:
         red_replay_alpha_blend_ptr(replay, &qxl->u.alpha_blend, flags);
@@ -952,6 +1000,9 @@ static QXLCompatDrawable *red_replay_compat_drawable(SpiceReplay *replay, uint32
     red_replay_rect_ptr(replay, "bitmap_area", &qxl->bitmap_area);
 
     replay_fscanf(replay, "type %d\n", &temp); qxl->type = temp;
+    if (replay->error) {
+        return NULL;
+    }
     switch (qxl->type) {
     case QXL_DRAW_ALPHA_BLEND:
         red_replay_alpha_blend_ptr_compat(replay, &qxl->u.alpha_blend, flags);
@@ -1022,6 +1073,9 @@ static QXLUpdateCmd *red_replay_update_cmd(SpiceReplay *replay)
     red_replay_rect_ptr(replay, "area", &qxl->area);
     replay_fscanf(replay, "update_id %d\n", &qxl->update_id);
     replay_fscanf(replay, "surface_id %d\n", &qxl->surface_id);
+    if (replay->error) {
+        return NULL;
+    }
     qxl->surface_id = replay_id_get(replay, qxl->surface_id);
 
     return qxl;
@@ -1047,6 +1101,9 @@ static QXLSurfaceCmd *red_replay_surface_cmd(SpiceReplay *replay)
     replay_fscanf(replay, "surface_id %d\n", &qxl->surface_id);
     replay_fscanf(replay, "type %d\n", &temp); qxl->type = temp;
     replay_fscanf(replay, "flags %d\n", &qxl->flags);
+    if (replay->error) {
+        return NULL;
+    }
 
     switch (qxl->type) {
     case QXL_SURFACE_CMD_CREATE:
@@ -1054,6 +1111,9 @@ static QXLSurfaceCmd *red_replay_surface_cmd(SpiceReplay *replay)
         replay_fscanf(replay, "u.surface_create.width %d\n", &qxl->u.surface_create.width);
         replay_fscanf(replay, "u.surface_create.height %d\n", &qxl->u.surface_create.height);
         replay_fscanf(replay, "u.surface_create.stride %d\n", &qxl->u.surface_create.stride);
+        if (replay->error) {
+            return NULL;
+        }
         size = qxl->u.surface_create.height * abs(qxl->u.surface_create.stride);
         if ((qxl->flags & QXL_SURF_FLAG_KEEP_DATA) != 0) {
             read_binary(replay, "data", &read_size, (uint8_t**)&qxl->u.surface_create.data, 0);
@@ -1100,6 +1160,9 @@ static QXLCursor *red_replay_cursor(SpiceReplay *replay)
     cursor.header.hot_spot_y = temp;
 
     replay_fscanf(replay, "data_size %d\n", &temp);
+    if (replay->error) {
+        return NULL;
+    }
     cursor.data_size = temp;
     cursor.data_size = red_replay_data_chunks(replay, "cursor", (uint8_t**)&qxl, sizeof(QXLCursor));
     qxl->header = cursor.header;
@@ -1114,6 +1177,9 @@ static QXLCursorCmd *red_replay_cursor_cmd(SpiceReplay *replay)
 
     replay_fscanf(replay, "cursor_cmd\n");
     replay_fscanf(replay, "type %d\n", &temp);
+    if (replay->error) {
+        return NULL;
+    }
     qxl->type = temp;
     switch (qxl->type) {
     case QXL_CURSOR_SET:
@@ -1163,6 +1229,9 @@ static void replay_handle_create_primary(QXLWorker *worker, SpiceReplay *replay)
         &surface.stride, &surface.format);
     replay_fscanf(replay, "%d %d %d %d\n", &surface.position, &surface.mouse_mode,
         &surface.flags, &surface.type);
+    if (replay->error) {
+        return;
+    }
     read_binary(replay, "data", &size, &mem, 0);
     surface.group_id = 0;
     surface.mem = QXLPHYSICAL_FROM_PTR(mem);
commit 98557452bc3ba8d3796a103ace31d6a17dd81225
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Thu Sep 15 17:11:29 2016 +0100

    replay: Record allocations in a GList to handle errors
    
    Allocations are kept into a GList to be able to free in case some
    errors happened.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index 7ce8f47..8229d5e 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -50,6 +50,8 @@ struct SpiceReplay {
     GArray *id_free; // free list
     int nsurfaces;
 
+    GList *allocated;
+
     pthread_mutex_t mutex;
     pthread_cond_t cond;
 };
@@ -88,6 +90,26 @@ static replay_t replay_fscanf(SpiceReplay *replay, const char *fmt, ...)
     return replay->error ? REPLAY_ERROR : REPLAY_OK;
 }
 
+static inline void *replay_malloc(SpiceReplay *replay, size_t size)
+{
+    void *mem = spice_malloc(size);
+    replay->allocated = g_list_prepend(replay->allocated, mem);
+    return mem;
+}
+
+static inline void *replay_malloc0(SpiceReplay *replay, size_t size)
+{
+    void *mem = replay_malloc(replay, size);
+    memset(mem, 0, size);
+    return mem;
+}
+
+static inline void replay_free(SpiceReplay *replay, void *mem)
+{
+    replay->allocated = g_list_remove(replay->allocated, mem);
+    free(mem);
+}
+
 static uint32_t replay_id_get(SpiceReplay *replay, uint32_t id)
 {
     uint32_t newid = 0;
@@ -199,7 +221,7 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
         return REPLAY_ERROR;
 
     if (*buf == NULL) {
-        *buf = spice_malloc(*size + base_size);
+        *buf = replay_malloc(replay, *size + base_size);
     }
 #if 0
     {
@@ -213,7 +235,7 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
         int ret;
 
         replay_fscanf(replay, "%d:", &zlib_size);
-        zlib_buffer = spice_malloc(zlib_size);
+        zlib_buffer = replay_malloc(replay, zlib_size);
         replay_fread(replay, zlib_buffer, zlib_size);
         strm.zalloc = Z_NULL;
         strm.zfree = Z_NULL;
@@ -241,7 +263,7 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
             }
         }
         (void)inflateEnd(&strm);
-        free(zlib_buffer); // TODO - avoid repeat alloc/dealloc by keeping last
+        replay_free(replay, zlib_buffer); // TODO - avoid repeat alloc/dealloc by keeping last
     } else {
         replay_fread(replay, *buf + base_size, *size);
     }
@@ -377,7 +399,7 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
         return NULL;
     }
 
-    qxl = spice_new0(QXLImage, 1);
+    qxl = (QXLImage*)replay_malloc0(replay, sizeof(QXLImage));
     replay_fscanf(replay, "descriptor.id %"PRIu64"\n", &qxl->descriptor.id);
     replay_fscanf(replay, "descriptor.type %d\n", &temp); qxl->descriptor.type = temp;
     replay_fscanf(replay, "descriptor.flags %d\n", &temp); qxl->descriptor.flags = temp;
@@ -398,7 +420,7 @@ static QXLImage *red_replay_image(SpiceReplay *replay, uint32_t flags)
             int i, num_ents;
 
             replay_fscanf(replay, "qp.num_ents %d\n", &num_ents);
-            qp = spice_malloc(sizeof(QXLPalette) + num_ents * sizeof(qp->ents[0]));
+            qp = replay_malloc(replay, sizeof(QXLPalette) + num_ents * sizeof(qp->ents[0]));
             qp->num_ents = num_ents;
             qxl->bitmap.palette = QXLPHYSICAL_FROM_PTR(qp);
             replay_fscanf(replay, "unique %"PRIu64"\n", &qp->unique);
@@ -788,7 +810,7 @@ static void red_replay_composite_free(SpiceReplay *replay, QXLComposite *qxl, ui
 
 static QXLDrawable *red_replay_native_drawable(SpiceReplay *replay, uint32_t flags)
 {
-    QXLDrawable *qxl = spice_malloc0(sizeof(QXLDrawable)); // TODO - this is too large usually
+    QXLDrawable *qxl = replay_malloc0(replay, sizeof(QXLDrawable)); // TODO - this is too large usually
     int i;
     int temp;
 
@@ -919,7 +941,7 @@ static void red_replay_native_drawable_free(SpiceReplay *replay, QXLDrawable *qx
 static QXLCompatDrawable *red_replay_compat_drawable(SpiceReplay *replay, uint32_t flags)
 {
     int temp;
-    QXLCompatDrawable *qxl = spice_malloc0(sizeof(QXLCompatDrawable)); // TODO - too large usually
+    QXLCompatDrawable *qxl = replay_malloc0(replay, sizeof(QXLCompatDrawable)); // TODO - too large usually
 
     red_replay_rect_ptr(replay, "bbox", &qxl->bbox);
     red_replay_clip_ptr(replay, &qxl->clip);
@@ -994,7 +1016,7 @@ static QXLPHYSICAL red_replay_drawable(SpiceReplay *replay, uint32_t flags)
 
 static QXLUpdateCmd *red_replay_update_cmd(SpiceReplay *replay)
 {
-    QXLUpdateCmd *qxl = spice_malloc0(sizeof(QXLUpdateCmd));
+    QXLUpdateCmd *qxl = replay_malloc0(replay, sizeof(QXLUpdateCmd));
 
     replay_fscanf(replay, "update\n");
     red_replay_rect_ptr(replay, "area", &qxl->area);
@@ -1019,7 +1041,7 @@ static QXLSurfaceCmd *red_replay_surface_cmd(SpiceReplay *replay)
     size_t size;
     size_t read_size;
     int temp;
-    QXLSurfaceCmd *qxl = spice_malloc0(sizeof(QXLSurfaceCmd));
+    QXLSurfaceCmd *qxl = replay_malloc0(replay, sizeof(QXLSurfaceCmd));
 
     replay_fscanf(replay, "surface_cmd\n");
     replay_fscanf(replay, "surface_id %d\n", &qxl->surface_id);
@@ -1039,7 +1061,7 @@ static QXLSurfaceCmd *red_replay_surface_cmd(SpiceReplay *replay)
                 spice_printerr("mismatch %zu != %zu", size, read_size);
             }
         } else {
-            qxl->u.surface_create.data = QXLPHYSICAL_FROM_PTR(spice_malloc(size));
+            qxl->u.surface_create.data = QXLPHYSICAL_FROM_PTR(replay_malloc(replay, size));
         }
         qxl->surface_id = replay_id_new(replay, qxl->surface_id);
         break;
@@ -1088,7 +1110,7 @@ static QXLCursor *red_replay_cursor(SpiceReplay *replay)
 static QXLCursorCmd *red_replay_cursor_cmd(SpiceReplay *replay)
 {
     int temp;
-    QXLCursorCmd *qxl = spice_new0(QXLCursorCmd, 1);
+    QXLCursorCmd *qxl = replay_malloc0(replay, sizeof(QXLCursorCmd));
 
     replay_fscanf(replay, "cursor_cmd\n");
     replay_fscanf(replay, "type %d\n", &temp);
@@ -1185,7 +1207,7 @@ static void replay_handle_dev_input(QXLWorker *worker, SpiceReplay *replay,
 SPICE_GNUC_VISIBLE QXLCommandExt* spice_replay_next_cmd(SpiceReplay *replay,
                                                          QXLWorker *worker)
 {
-    QXLCommandExt* cmd;
+    QXLCommandExt* cmd = NULL;
     uint64_t timestamp;
     int type;
     int what = -1;
@@ -1195,13 +1217,13 @@ SPICE_GNUC_VISIBLE QXLCommandExt* spice_replay_next_cmd(SpiceReplay *replay,
         replay_fscanf(replay, "event %d %d %d %"PRIu64"\n", &counter,
                             &what, &type, &timestamp);
         if (replay->error) {
-            return NULL;
+            goto error;
         }
         if (what == 1) {
             replay_handle_dev_input(worker, replay, type);
         }
     }
-    cmd = g_new(QXLCommandExt, 1);
+    cmd = replay_malloc0(replay, sizeof(QXLCommandExt));
     cmd->cmd.type = type;
     cmd->group_id = 0;
     spice_debug("command %"PRIu64", %d\r", timestamp, cmd->cmd.type);
@@ -1224,6 +1246,10 @@ SPICE_GNUC_VISIBLE QXLCommandExt* spice_replay_next_cmd(SpiceReplay *replay,
         break;
     }
 
+    if (replay->error) {
+        goto error;
+    }
+
     QXLReleaseInfo *info;
     switch (cmd->cmd.type) {
     case QXL_CMD_DRAW:
@@ -1234,9 +1260,25 @@ SPICE_GNUC_VISIBLE QXLCommandExt* spice_replay_next_cmd(SpiceReplay *replay,
         info->id = (uintptr_t)cmd;
     }
 
+    /* all buffer allocated will be used by the caller but
+     * free the list of buffer allocated to avoid to free on next calls */
+    if (replay->allocated) {
+        g_list_free(replay->allocated);
+        replay->allocated = NULL;
+    }
+
     replay->counter++;
 
     return cmd;
+
+error:
+    /* if there were some allocation during the read free all
+     * buffers allocated to avoid leaks */
+    if (replay->allocated) {
+        g_list_free_full(replay->allocated, free);
+        replay->allocated = NULL;
+    }
+    return NULL;
 }
 
 SPICE_GNUC_VISIBLE void spice_replay_free_cmd(SpiceReplay *replay, QXLCommandExt *cmd)
@@ -1271,7 +1313,7 @@ SPICE_GNUC_VISIBLE void spice_replay_free_cmd(SpiceReplay *replay, QXLCommandExt
         break;
     }
 
-    g_free(cmd);
+    free(cmd);
 }
 
 /* caller is incharge of closing the replay when done and releasing the SpiceReplay
@@ -1305,6 +1347,7 @@ SpiceReplay *spice_replay_new(FILE *file, int nsurfaces)
     replay->id_map_inv = g_array_new(FALSE, FALSE, sizeof(uint32_t));
     replay->id_free = g_array_new(FALSE, FALSE, sizeof(uint32_t));
     replay->nsurfaces = nsurfaces;
+    replay->allocated = NULL;
 
     /* reserve id 0 */
     replay_id_new(replay, 0);
@@ -1316,6 +1359,7 @@ SPICE_GNUC_VISIBLE void spice_replay_free(SpiceReplay *replay)
 {
     spice_return_if_fail(replay != NULL);
 
+    g_list_free_full(replay->allocated, free);
     pthread_mutex_destroy(&replay->mutex);
     pthread_cond_destroy(&replay->cond);
     g_array_free(replay->id_map, TRUE);


More information about the Spice-commits mailing list