[Spice-commits] 6 commits - server/dispatcher.c server/display-channel.c server/image-encoders.c server/red-parse-qxl.c server/red-record-qxl.c

Frediano Ziglio fziglio at kemper.freedesktop.org
Tue Dec 6 16:46:06 UTC 2016


 server/dispatcher.c      |    2 +-
 server/display-channel.c |   12 ++++++------
 server/image-encoders.c  |    8 ++++++--
 server/red-parse-qxl.c   |    9 ++++++---
 server/red-record-qxl.c  |   10 ++++------
 5 files changed, 23 insertions(+), 18 deletions(-)

New commits:
commit 51d0ed6abb8d0d8ea2434431815a6d9b4e85f109
Author: Uri Lublin <uril at redhat.com>
Date:   Tue Dec 6 18:06:31 2016 +0200

    dispatcher: write_safe: move EINTR debug message
    
    spice_debug was called for not-EINTR case, move
    it to the right place.
    
    Signed-off-by: Uri Lublin <uril at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/dispatcher.c b/server/dispatcher.c
index f0479aa..f4fe97b 100644
--- a/server/dispatcher.c
+++ b/server/dispatcher.c
@@ -254,9 +254,9 @@ static int write_safe(int fd, uint8_t *buf, size_t size)
         ret = write(fd, buf + written_size, size - written_size);
         if (ret == -1) {
             if (errno != EINTR) {
-                spice_debug("EINTR in write");
                 return -1;
             }
+            spice_debug("EINTR in write");
             continue;
         }
         written_size += ret;
commit c3d5689f4a0787fc7ed671ea2e3cb68795b440ad
Author: Uri Lublin <uril at redhat.com>
Date:   Tue Dec 6 18:06:30 2016 +0200

    red-record-qxl: child_output_setup: remove fcntl call
    
    man 2 dup2 specifies:
      The close-on-exec flag (FD_CLOEXEC; see  fcntl(2))  for
      the duplicate descriptor is off.
    
    Since the purpose of the fcntl call is to turn off FD_CLOEXEC
    flag, and it's already done, just remove this call.
    
    Suggested-by: Frediano Ziglio <fziglio at redhat.com>
    Signed-off-by: Uri Lublin <uril at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/red-record-qxl.c b/server/red-record-qxl.c
index 184d6b9..ee22236 100644
--- a/server/red-record-qxl.c
+++ b/server/red-record-qxl.c
@@ -21,7 +21,6 @@
 
 #include <stdbool.h>
 #include <inttypes.h>
-#include <fcntl.h>
 #include <glib.h>
 #include "red-common.h"
 #include "memslot.h"
@@ -851,9 +850,6 @@ static void child_output_setup(gpointer user_data)
         continue;
     }
     close(fd);
-
-    // make sure file is not closed calling exec()
-    fcntl(STDOUT_FILENO, F_SETFD, 0);
 }
 
 RedRecord *red_record_new(const char *filename)
commit 4be1a6ec8c794f6847cb21f076f6f4dcbfb5862f
Author: Uri Lublin <uril at redhat.com>
Date:   Tue Dec 6 18:06:29 2016 +0200

    red-record-qxl: add curly braces to empty while loop
    
    Spice coding style suggests to use curly braces
    for while blocks.
    
    Some prefer the block to not be empty so continue
    is untouched.
    
    Signed-off-by: Uri Lublin <uril at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/red-record-qxl.c b/server/red-record-qxl.c
index 14a68a7..184d6b9 100644
--- a/server/red-record-qxl.c
+++ b/server/red-record-qxl.c
@@ -847,8 +847,9 @@ static void child_output_setup(gpointer user_data)
 {
     int fd = GPOINTER_TO_INT(user_data);
 
-    while (dup2(fd, STDOUT_FILENO) < 0 && errno == EINTR)
+    while (dup2(fd, STDOUT_FILENO) < 0 && errno == EINTR) {
         continue;
+    }
     close(fd);
 
     // make sure file is not closed calling exec()
@@ -890,8 +891,9 @@ RedRecord *red_record_new(const char *filename)
             fclose(f);
             spice_error("failed to setup filter for replay");
         }
-        while (dup2(fd_in, fileno(f)) < 0 && errno == EINTR)
+        while (dup2(fd_in, fileno(f)) < 0 && errno == EINTR) {
             continue;
+        }
         close(fd_in);
     }
 
commit 547f9f438701653474c825d9addb2af7af7e1be2
Author: Uri Lublin <uril at redhat.com>
Date:   Tue Dec 6 18:06:28 2016 +0200

    red_get_image_data_flat: allocate mem after sanity check
    
    This patch prevents possible memory leak.
    
    Found by coverity.
    
    Signed-off-by: Uri Lublin <uril at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/red-parse-qxl.c b/server/red-parse-qxl.c
index 11da3a1..89cb120 100644
--- a/server/red-parse-qxl.c
+++ b/server/red-parse-qxl.c
@@ -371,13 +371,16 @@ static SpiceChunks *red_get_image_data_flat(RedMemSlotInfo *slots, int group_id,
 {
     SpiceChunks *data;
     int error;
+    unsigned long bitmap_virt;
 
-    data = spice_chunks_new(1);
-    data->data_size      = size;
-    data->chunk[0].data  = (void*)memslot_get_virt(slots, addr, size, group_id, &error);
+    bitmap_virt = memslot_get_virt(slots, addr, size, group_id, &error);
     if (error) {
         return 0;
     }
+
+    data = spice_chunks_new(1);
+    data->data_size      = size;
+    data->chunk[0].data  = (void*)bitmap_virt;
     data->chunk[0].len   = size;
     return data;
 }
commit dad108edb156c889a63ee7f4d7a2844d6cf0db97
Author: Uri Lublin <uril at redhat.com>
Date:   Tue Dec 6 18:06:27 2016 +0200

    image_encoders: check shared_dict before accessing it
    
    In both image_encoders_restore_glz_dictionary() and
    image_encoders_get_glz_dictionary() shared-dict may
    be NULL if size is too large, and the server gets
    size from the network.
    
    Both functions end up calling glz_enc_dictionary_create()
    that calls glz_dictionary_window_create() where size is
    checked.
    
    Found by coverity.
    
    Signed-off-by: Uri Lublin <uril at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/image-encoders.c b/server/image-encoders.c
index 3a73e0c..0d57260 100644
--- a/server/image-encoders.c
+++ b/server/image-encoders.c
@@ -746,7 +746,9 @@ gboolean image_encoders_get_glz_dictionary(ImageEncoders *enc,
         shared_dict->refs++;
     } else {
         shared_dict = create_glz_dictionary(enc, client, id, window_size);
-        glz_dictionary_list = g_list_prepend(glz_dictionary_list, shared_dict);
+        if (shared_dict != NULL) {
+            glz_dictionary_list = g_list_prepend(glz_dictionary_list, shared_dict);
+        }
     }
 
     pthread_mutex_unlock(&glz_dictionary_list_lock);
@@ -782,7 +784,9 @@ gboolean image_encoders_restore_glz_dictionary(ImageEncoders *enc,
         shared_dict->refs++;
     } else {
         shared_dict = restore_glz_dictionary(enc, client, id, restore_data);
-        glz_dictionary_list = g_list_prepend(glz_dictionary_list, shared_dict);
+        if(shared_dict != NULL) {
+            glz_dictionary_list = g_list_prepend(glz_dictionary_list, shared_dict);
+        }
     }
 
     pthread_mutex_unlock(&glz_dictionary_list_lock);
commit a286da42d2376b6eb22b9022069e44fe6faba5b7
Author: Uri Lublin <uril at redhat.com>
Date:   Tue Dec 6 18:06:26 2016 +0200

    display-channel: current_remove: rename inner variable 'container'
    
    It shadows the outer one.
    
    Renamed also the outer 'container' variable.
    
    Signed-off-by: Uri Lublin <uril at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/display-channel.c b/server/display-channel.c
index 132f5a0..6069883 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -428,7 +428,7 @@ static void current_remove(DisplayChannel *display, TreeItem *item)
 
     /* depth-first tree traversal, TODO: do a to tree_foreach()? */
     for (;;) {
-        Container *container = now->container;
+        Container *container_of_now = now->container;
         RingItem *ring_item;
 
         if (now->type == TREE_ITEM_TYPE_DRAWABLE) {
@@ -437,25 +437,25 @@ static void current_remove(DisplayChannel *display, TreeItem *item)
             drawable_remove_from_pipes(drawable);
             current_remove_drawable(display, drawable);
         } else {
-            Container *container = CONTAINER(now);
+            Container *now_as_container = CONTAINER(now);
 
             spice_assert(now->type == TREE_ITEM_TYPE_CONTAINER);
 
-            if ((ring_item = ring_get_head(&container->items))) {
+            if ((ring_item = ring_get_head(&now_as_container->items))) {
                 now = SPICE_CONTAINEROF(ring_item, TreeItem, siblings_link);
                 continue;
             }
             ring_item = now->siblings_link.prev;
-            container_free(container);
+            container_free(now_as_container);
         }
         if (now == item) {
             return;
         }
 
-        if ((ring_item = ring_next(&container->items, ring_item))) {
+        if ((ring_item = ring_next(&container_of_now->items, ring_item))) {
             now = SPICE_CONTAINEROF(ring_item, TreeItem, siblings_link);
         } else {
-            now = &container->base;
+            now = &container_of_now->base;
         }
     }
 }


More information about the Spice-commits mailing list