[Spice-devel] [PATCH 15/16] worker: simplify RedCompressBuf

Frediano Ziglio fziglio at redhat.com
Tue Nov 10 06:16:28 PST 2015


From: Christophe Fergeau <cfergeau at redhat.com>

Make sure an allocated buffer is correctly referenced by the marshaller,
and can't be free and reused by mistake. Simplify the code by using
GSlice
---
 server/display-channel.h |   3 --
 server/red_worker.c      | 117 ++++++++++++-----------------------------------
 2 files changed, 28 insertions(+), 92 deletions(-)

diff --git a/server/display-channel.h b/server/display-channel.h
index f1f4e3a..f45860c 100644
--- a/server/display-channel.h
+++ b/server/display-channel.h
@@ -73,7 +73,6 @@
 typedef struct RedCompressBuf RedCompressBuf;
 struct RedCompressBuf {
     uint8_t buf[64 * 1024];
-    RedCompressBuf *next;
     RedCompressBuf *send_next;
 };
 
@@ -179,8 +178,6 @@ struct DisplayChannelClient {
         uint32_t stream_outbuf_size;
         uint8_t *stream_outbuf; // caution stream buffer is also used as compress bufs!!!
 
-        RedCompressBuf *used_compress_bufs;
-
         FreeList free_list;
         uint64_t pixmap_cache_items[MAX_DRAWABLE_PIXMAP_CACHE_ITEMS];
         int num_pixmap_cache_items;
diff --git a/server/red_worker.c b/server/red_worker.c
index 5daca87..e0d9593 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -3462,16 +3462,27 @@ static void red_push_surface_image(DisplayChannelClient *dcc, int surface_id)
     red_channel_client_push(RED_CHANNEL_CLIENT(dcc));
 }
 
+static RedCompressBuf *compress_buf_new(void)
+{
+    return g_slice_new(RedCompressBuf);
+}
+
+static void compress_buf_free(RedCompressBuf *buf)
+{
+    g_slice_free(RedCompressBuf, buf);
+}
+
 static void marshaller_add_compressed(SpiceMarshaller *m,
                                       RedCompressBuf *comp_buf, size_t size)
 {
     size_t max = size;
     size_t now;
     do {
-        spice_assert(comp_buf);
+        spice_return_if_fail(comp_buf);
         now = MIN(sizeof(comp_buf->buf), max);
         max -= now;
-        spice_marshaller_add_ref(m, (uint8_t*)comp_buf->buf, now);
+        spice_marshaller_add_ref_full(m, (uint8_t*)comp_buf->buf, now,
+                                      (spice_marshaller_item_free_func)compress_buf_free, NULL);
         comp_buf = comp_buf->send_next;
     } while (max);
 }
@@ -3506,66 +3517,6 @@ static inline void fill_palette(DisplayChannelClient *dcc,
     }
 }
 
-static inline RedCompressBuf *red_display_alloc_compress_buf(DisplayChannelClient *dcc)
-{
-    DisplayChannel *display_channel = DCC_TO_DC(dcc);
-    RedCompressBuf *ret;
-
-    if (display_channel->free_compress_bufs) {
-        ret = display_channel->free_compress_bufs;
-        display_channel->free_compress_bufs = ret->next;
-    } else {
-        ret = spice_new(RedCompressBuf, 1);
-    }
-
-    ret->next = dcc->send_data.used_compress_bufs;
-    dcc->send_data.used_compress_bufs = ret;
-    return ret;
-}
-
-static inline void __red_display_free_compress_buf(DisplayChannel *dc,
-                                                   RedCompressBuf *buf)
-{
-    buf->next = dc->free_compress_bufs;
-    dc->free_compress_bufs = buf;
-}
-
-static void red_display_free_compress_buf(DisplayChannelClient *dcc,
-                                          RedCompressBuf *buf)
-{
-    DisplayChannel *display_channel = DCC_TO_DC(dcc);
-    RedCompressBuf **curr_used = &dcc->send_data.used_compress_bufs;
-
-    for (;;) {
-        spice_assert(*curr_used);
-        if (*curr_used == buf) {
-            *curr_used = buf->next;
-            break;
-        }
-        curr_used = &(*curr_used)->next;
-    }
-    __red_display_free_compress_buf(display_channel, buf);
-}
-
-static void red_display_reset_compress_buf(DisplayChannelClient *dcc)
-{
-    while (dcc->send_data.used_compress_bufs) {
-        RedCompressBuf *buf = dcc->send_data.used_compress_bufs;
-        dcc->send_data.used_compress_bufs = buf->next;
-        __red_display_free_compress_buf(DCC_TO_DC(dcc), buf);
-    }
-}
-
-static void red_display_destroy_compress_bufs(DisplayChannel *display_channel)
-{
-    spice_assert(!red_channel_is_connected(RED_CHANNEL(display_channel)));
-    while (display_channel->free_compress_bufs) {
-        RedCompressBuf *buf = display_channel->free_compress_bufs;
-        display_channel->free_compress_bufs = buf->next;
-        free(buf);
-    }
-}
-
 /******************************************************
  *      Global lz red drawables routines
 *******************************************************/
@@ -3890,9 +3841,7 @@ static int encoder_usr_more_space(EncoderData *enc_data, uint8_t **io_ptr)
 {
     RedCompressBuf *buf;
 
-    if (!(buf = red_display_alloc_compress_buf(enc_data->dcc))) {
-        return 0;
-    }
+    buf = compress_buf_new();
     enc_data->bufs_tail->send_next = buf;
     enc_data->bufs_tail = buf;
     buf->send_next = NULL;
@@ -4152,7 +4101,7 @@ static inline int red_glz_compress_image(DisplayChannelClient *dcc,
     int glz_size;
     int zlib_size;
 
-    glz_data->data.bufs_tail = red_display_alloc_compress_buf(dcc);
+    glz_data->data.bufs_tail = compress_buf_new();
     glz_data->data.bufs_head = glz_data->data.bufs_tail;
 
     if (!glz_data->data.bufs_head) {
@@ -4173,7 +4122,7 @@ static inline int red_glz_compress_image(DisplayChannelClient *dcc,
 
     glz_size = glz_encode(dcc->glz, type, src->x, src->y,
                           (src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN), NULL, 0,
-                          src->stride, (uint8_t*)glz_data->data.bufs_head->buf,
+                          src->stride, glz_data->data.bufs_head->buf,
                           sizeof(glz_data->data.bufs_head->buf),
                           glz_drawable_instance,
                           &glz_drawable_instance->glz_instance);
@@ -4188,7 +4137,7 @@ static inline int red_glz_compress_image(DisplayChannelClient *dcc,
 #endif
     zlib_data = &worker->zlib_data;
 
-    zlib_data->data.bufs_tail = red_display_alloc_compress_buf(dcc);
+    zlib_data->data.bufs_tail = compress_buf_new();
     zlib_data->data.bufs_head = zlib_data->data.bufs_tail;
 
     if (!zlib_data->data.bufs_head) {
@@ -4203,7 +4152,7 @@ static inline int red_glz_compress_image(DisplayChannelClient *dcc,
     zlib_data->data.u.compressed_data.size_left = glz_size;
 
     zlib_size = zlib_encode(worker->zlib, display_channel->zlib_level,
-                            glz_size, (uint8_t*)zlib_data->data.bufs_head->buf,
+                            glz_size, zlib_data->data.bufs_head->buf,
                             sizeof(zlib_data->data.bufs_head->buf));
 
     // the compressed buffer is bigger than the original data
@@ -4211,7 +4160,7 @@ static inline int red_glz_compress_image(DisplayChannelClient *dcc,
         while (zlib_data->data.bufs_head) {
             RedCompressBuf *buf = zlib_data->data.bufs_head;
             zlib_data->data.bufs_head = buf->send_next;
-            red_display_free_compress_buf(dcc, buf);
+            compress_buf_free(buf);
         }
         goto glz;
     }
@@ -4250,7 +4199,7 @@ static inline int red_lz_compress_image(DisplayChannelClient *dcc,
     stat_time_t start_time = stat_now(worker);
 #endif
 
-    lz_data->data.bufs_tail = red_display_alloc_compress_buf(dcc);
+    lz_data->data.bufs_tail = compress_buf_new();
     lz_data->data.bufs_head = lz_data->data.bufs_tail;
 
     if (!lz_data->data.bufs_head) {
@@ -4264,7 +4213,7 @@ static inline int red_lz_compress_image(DisplayChannelClient *dcc,
         while (lz_data->data.bufs_head) {
             RedCompressBuf *buf = lz_data->data.bufs_head;
             lz_data->data.bufs_head = buf->send_next;
-            red_display_free_compress_buf(dcc, buf);
+            compress_buf_free(buf);
         }
         return FALSE;
     }
@@ -4278,7 +4227,7 @@ static inline int red_lz_compress_image(DisplayChannelClient *dcc,
     size = lz_encode(lz, type, src->x, src->y,
                      !!(src->flags & SPICE_BITMAP_FLAGS_TOP_DOWN),
                      NULL, 0, src->stride,
-                     (uint8_t*)lz_data->data.bufs_head->buf,
+                     lz_data->data.bufs_head->buf,
                      sizeof(lz_data->data.bufs_head->buf));
 
     // the compressed buffer is bigger than the original data
@@ -4353,7 +4302,7 @@ static int red_jpeg_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
         return FALSE;
     }
 
-    jpeg_data->data.bufs_tail = red_display_alloc_compress_buf(dcc);
+    jpeg_data->data.bufs_tail = compress_buf_new();
     jpeg_data->data.bufs_head = jpeg_data->data.bufs_tail;
 
     if (!jpeg_data->data.bufs_head) {
@@ -4368,7 +4317,7 @@ static int red_jpeg_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
         while (jpeg_data->data.bufs_head) {
             RedCompressBuf *buf = jpeg_data->data.bufs_head;
             jpeg_data->data.bufs_head = buf->send_next;
-            red_display_free_compress_buf(dcc, buf);
+            compress_buf_free(buf);
         }
         return FALSE;
     }
@@ -4391,7 +4340,7 @@ static int red_jpeg_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
     }
     jpeg_size = jpeg_encode(jpeg, display_channel->jpeg_quality, jpeg_in_type,
                             src->x, src->y, NULL,
-                            0, stride, (uint8_t*)jpeg_data->data.bufs_head->buf,
+                            0, stride, jpeg_data->data.bufs_head->buf,
                             sizeof(jpeg_data->data.bufs_head->buf));
 
     // the compressed buffer is bigger than the original data
@@ -4417,7 +4366,7 @@ static int red_jpeg_compress_image(DisplayChannelClient *dcc, SpiceImage *dest,
 
     comp_head_filled = jpeg_size % sizeof(lz_data->data.bufs_head->buf);
     comp_head_left = sizeof(lz_data->data.bufs_head->buf) - comp_head_filled;
-    lz_out_start_byte = ((uint8_t *)lz_data->data.bufs_head->buf) + comp_head_filled;
+    lz_out_start_byte = lz_data->data.bufs_head->buf + comp_head_filled;
 
     lz_data->data.dcc = dcc;
 
@@ -4553,13 +4502,8 @@ static inline int red_quic_compress_image(DisplayChannelClient *dcc, SpiceImage
         return FALSE;
     }
 
-    quic_data->data.bufs_tail = red_display_alloc_compress_buf(dcc);
+    quic_data->data.bufs_tail = compress_buf_new();
     quic_data->data.bufs_head = quic_data->data.bufs_tail;
-
-    if (!quic_data->data.bufs_head) {
-        return FALSE;
-    }
-
     quic_data->data.bufs_head->send_next = NULL;
     quic_data->data.dcc = dcc;
 
@@ -4567,7 +4511,7 @@ static inline int red_quic_compress_image(DisplayChannelClient *dcc, SpiceImage
         while (quic_data->data.bufs_head) {
             RedCompressBuf *buf = quic_data->data.bufs_head;
             quic_data->data.bufs_head = buf->send_next;
-            red_display_free_compress_buf(dcc, buf);
+            compress_buf_free(buf);
         }
         return FALSE;
     }
@@ -5047,7 +4991,6 @@ static void fill_attr(SpiceMarshaller *m, SpiceLineAttr *attr, uint32_t group_id
 
 static inline void red_display_reset_send_data(DisplayChannelClient *dcc)
 {
-    red_display_reset_compress_buf(dcc);
     dcc->send_data.free_list.res->count = 0;
     dcc->send_data.num_pixmap_cache_items = 0;
     memset(dcc->send_data.free_list.sync, 0, sizeof(dcc->send_data.free_list.sync));
@@ -7274,14 +7217,10 @@ static void display_channel_client_on_disconnect(RedChannelClient *rcc)
     red_release_glz(dcc);
     red_reset_palette_cache(dcc);
     free(dcc->send_data.stream_outbuf);
-    red_display_reset_compress_buf(dcc);
     free(dcc->send_data.free_list.res);
     dcc_destroy_stream_agents(dcc);
 
     // this was the last channel client
-    if (!red_channel_is_connected(rcc->channel)) {
-        red_display_destroy_compress_bufs(display);
-    }
     spice_debug("#draw=%d, #red_draw=%d, #glz_draw=%d",
                 display->drawable_count, worker->red_drawable_count,
                 worker->glz_drawable_count);
-- 
2.4.3



More information about the Spice-devel mailing list