[Spice-devel] [PATCH v3 1/8] replay: Record allocations in a GList to handle errors

Frediano Ziglio fziglio at redhat.com
Fri Sep 16 11:32:54 UTC 2016


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>
---
 server/red-replay-qxl.c | 68 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 12 deletions(-)

diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index 7ce8f47..7eddaf4 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,20 @@ 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 uint32_t replay_id_get(SpiceReplay *replay, uint32_t id)
 {
     uint32_t newid = 0;
@@ -199,7 +215,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
     {
@@ -211,9 +227,11 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
     spice_return_val_if_fail(with_zlib != -1, REPLAY_ERROR);
     if (with_zlib) {
         int ret;
+        GList *elem;
 
         replay_fscanf(replay, "%d:", &zlib_size);
-        zlib_buffer = spice_malloc(zlib_size);
+        zlib_buffer = replay_malloc(replay, zlib_size);
+        elem = replay->allocated;
         replay_fread(replay, zlib_buffer, zlib_size);
         strm.zalloc = Z_NULL;
         strm.zfree = Z_NULL;
@@ -241,6 +259,7 @@ static replay_t read_binary(SpiceReplay *replay, const char *prefix, size_t *siz
             }
         }
         (void)inflateEnd(&strm);
+        replay->allocated = g_list_remove_link(replay->allocated, elem);
         free(zlib_buffer); // TODO - avoid repeat alloc/dealloc by keeping last
     } else {
         replay_fread(replay, *buf + base_size, *size);
@@ -377,7 +396,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 +417,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 +807,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 +938,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 +1013,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 +1038,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 +1058,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 +1107,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 +1204,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,7 +1214,7 @@ 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);
@@ -1224,6 +1243,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 +1257,28 @@ 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;
+    }
+    if (cmd) {
+        g_free(cmd);
+    }
+    return NULL;
 }
 
 SPICE_GNUC_VISIBLE void spice_replay_free_cmd(SpiceReplay *replay, QXLCommandExt *cmd)
@@ -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);
-- 
2.7.4



More information about the Spice-devel mailing list