[Spice-devel] [PATCH v3 5/8] replay: Update pointer in allocated list

[Jonathon Jongsma]

On Fri, 2016-09-16 at 12:32 +0100, Frediano Ziglio wrote:
> Avoid to free invalid pointer.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/red-replay-qxl.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
> index 5fcb243..6914c17 100644
> --- a/server/red-replay-qxl.c
> +++ b/server/red-replay-qxl.c
> @@ -413,6 +413,7 @@ static QXLImage *red_replay_image(SpiceReplay
> *replay, uint32_t flags)
>      int temp;
>      int has_palette;
>      int has_image;
> +    GList *elem;
>  
>      replay_fscanf(replay, "image %d\n", &has_image);
>      if (replay->error) {
> @@ -423,6 +424,7 @@ static QXLImage *red_replay_image(SpiceReplay
> *replay, uint32_t flags)
>      }
>  
>      qxl = (QXLImage*)replay_malloc0(replay, sizeof(QXLImage));
> +    elem = replay->allocated;
>      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;
> @@ -485,8 +487,9 @@ static QXLImage *red_replay_image(SpiceReplay
> *replay, uint32_t flags)
>          if (replay->error) {
>              return NULL;
>          }
> -        qxl = realloc(qxl, sizeof(QXLImageDescriptor) +
> sizeof(QXLQUICData) +
> -                      qxl->quic.data_size);
> +        qxl = spice_realloc(qxl, sizeof(QXLImageDescriptor) +
> sizeof(QXLQUICData) +
> +                            qxl->quic.data_size);
> +        elem->data = qxl;
>          size = red_replay_data_chunks(replay, "quic.data",
> (uint8_t**)&qxl->quic.data, 0);
>          spice_assert(size == qxl->quic.data_size);
>          break;


Again, this feels like an encapsulation violation. The calling code
needs to know that the replay_malloc prepends it to the list. Something
like the following patch feels a bit cleaner to me:



diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c
index 97c1c6f..bf96bb1 100644
--- a/server/red-replay-qxl.c
+++ b/server/red-replay-qxl.c
@@ -109,6 +109,13 @@ static inline void *replay_malloc0(SpiceReplay
*replay, size_t size)
     return mem;
 }
 
+static inline void *replay_realloc(SpiceReplay *replay, void *mem,
size_t n_bytes)
+{
+    GList *elem = g_list_find(replay->allocated, mem);
+    elem->data = spice_realloc(mem, n_bytes);
+    return elem->data;
+}
+
 static uint32_t replay_id_get(SpiceReplay *replay, uint32_t id)
 {
     uint32_t newid = 0;
@@ -413,7 +420,6 @@ static QXLImage *red_replay_image(SpiceReplay
*replay, uint32_t flags)
     int temp;
     int has_palette;
     int has_image;
-    GList *elem;
 
     replay_fscanf(replay, "image %d\n", &has_image);
     if (replay->error) {
@@ -424,7 +430,6 @@ static QXLImage *red_replay_image(SpiceReplay
*replay, uint32_t flags)
     }
 
     qxl = (QXLImage*)replay_malloc0(replay, sizeof(QXLImage));
-    elem = replay->allocated;
     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;
@@ -487,9 +492,8 @@ static QXLImage *red_replay_image(SpiceReplay
*replay, uint32_t flags)
         if (replay->error) {
             return NULL;
         }
-        qxl = spice_realloc(qxl, sizeof(QXLImageDescriptor) +
sizeof(QXLQUICData) +
-                            qxl->quic.data_size);
-        elem->data = qxl;
+        qxl = replay_realloc(replay, qxl, sizeof(QXLImageDescriptor) +
sizeof(QXLQUICData) +
+                             qxl->quic.data_size);
         size = red_replay_data_chunks(replay, "quic.data",
(uint8_t**)&qxl->quic.data, 0);
         spice_assert(size == qxl->quic.data_size);
         break;