[Spice-commits] 2 commits - server/red-worker.c

[Frediano Ziglio]

 server/red-worker.c |   46 ++++++++++++++++++++++------------------------
 1 file changed, 22 insertions(+), 24 deletions(-)

New commits:
commit 90f3655ad46b037978a29e74d4b57f421acc1dd8
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Mon Dec 19 07:51:38 2016 +0000

    red-worker: Do not leak memory for surface commands
    
    This happened during VM resume.
    RedSurfaceCmd were allocated but never freed.
    We don't need to malloc the RedSurfaceCmd used in handle_dev_close()
    as display_channel_process_surface_cmd() will not try to reference
    it after it has returned.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/server/red-worker.c b/server/red-worker.c
index 1cd6250..394a935 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -984,19 +984,19 @@ static void handle_dev_close(void *opaque, void *payload)
 
 static int loadvm_command(RedWorker *worker, QXLCommandExt *ext)
 {
-    RedSurfaceCmd *surface_cmd;
+    RedSurfaceCmd surface_cmd;
 
     switch (ext->cmd.type) {
     case QXL_CMD_CURSOR:
         return red_process_cursor_cmd(worker, ext);
 
     case QXL_CMD_SURFACE:
-        surface_cmd = spice_new0(RedSurfaceCmd, 1);
-        if (red_get_surface_cmd(&worker->mem_slots, ext->group_id, surface_cmd, ext->cmd.data)) {
-            free(surface_cmd);
+        if (red_get_surface_cmd(&worker->mem_slots, ext->group_id, &surface_cmd, ext->cmd.data)) {
             return FALSE;
         }
-        display_channel_process_surface_cmd(worker->display_channel, surface_cmd, TRUE);
+        display_channel_process_surface_cmd(worker->display_channel, &surface_cmd, TRUE);
+        // do not release resource as is released inside display_channel_process_surface_cmd
+        red_put_surface_cmd(&surface_cmd);
         break;
     default:
         spice_warning("unhandled loadvm command type (%d)", ext->cmd.type);
commit e36e700d17db558804c9d75bc848cf263b611e28
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Mon Dec 19 07:49:59 2016 +0000

    red-worker: Reuse code to process cursor command
    
    Code to read and process cursor commands were the same
    so use a common function for better reuse.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/server/red-worker.c b/server/red-worker.c
index 9fd565f..1cd6250 100644
--- a/server/red-worker.c
+++ b/server/red-worker.c
@@ -104,6 +104,19 @@ void red_drawable_unref(RedDrawable *red_drawable)
     free(red_drawable);
 }
 
+static gboolean red_process_cursor_cmd(RedWorker *worker, const QXLCommandExt *ext)
+{
+    RedCursorCmd *cursor_cmd;
+
+    cursor_cmd = spice_new0(RedCursorCmd, 1);
+    if (red_get_cursor_cmd(&worker->mem_slots, ext->group_id, cursor_cmd, ext->cmd.data)) {
+        free(cursor_cmd);
+        return FALSE;
+    }
+    cursor_channel_process_cmd(worker->cursor_channel, cursor_cmd);
+    return TRUE;
+}
+
 static int red_process_cursor(RedWorker *worker, int *ring_is_empty)
 {
     QXLCommandExt ext_cmd;
@@ -134,18 +147,9 @@ static int red_process_cursor(RedWorker *worker, int *ring_is_empty)
 
         worker->cursor_poll_tries = 0;
         switch (ext_cmd.cmd.type) {
-        case QXL_CMD_CURSOR: {
-            RedCursorCmd *cursor = spice_new0(RedCursorCmd, 1);
-
-            if (red_get_cursor_cmd(&worker->mem_slots, ext_cmd.group_id,
-                                    cursor, ext_cmd.cmd.data)) {
-                free(cursor);
-                break;
-            }
-
-            cursor_channel_process_cmd(worker->cursor_channel, cursor);
+        case QXL_CMD_CURSOR:
+            red_process_cursor_cmd(worker, &ext_cmd);
             break;
-        }
         default:
             spice_warning("bad command type");
         }
@@ -980,18 +984,12 @@ static void handle_dev_close(void *opaque, void *payload)
 
 static int loadvm_command(RedWorker *worker, QXLCommandExt *ext)
 {
-    RedCursorCmd *cursor_cmd;
     RedSurfaceCmd *surface_cmd;
 
     switch (ext->cmd.type) {
     case QXL_CMD_CURSOR:
-        cursor_cmd = spice_new0(RedCursorCmd, 1);
-        if (red_get_cursor_cmd(&worker->mem_slots, ext->group_id, cursor_cmd, ext->cmd.data)) {
-            free(cursor_cmd);
-            return FALSE;
-        }
-        cursor_channel_process_cmd(worker->cursor_channel, cursor_cmd);
-        break;
+        return red_process_cursor_cmd(worker, ext);
+
     case QXL_CMD_SURFACE:
         surface_cmd = spice_new0(RedSurfaceCmd, 1);
         if (red_get_surface_cmd(&worker->mem_slots, ext->group_id, surface_cmd, ext->cmd.data)) {