diff mbox

[PULL,4/8] spice: reset cursor on resize

Message ID 1456237462-3687-5-git-send-email-kraxel@redhat.com
State New
Headers show

Commit Message

Gerd Hoffmann Feb. 23, 2016, 2:24 p.m. UTC 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Spice server will clear the cursor on resize. QXL driver reset it after
resize, however, virtio and other devices do not. Teach qemu to set it
back.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/spice-display.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Comments

Paolo Bonzini Feb. 25, 2016, 11:10 p.m. UTC | #1 
On 23/02/2016 15:24, Gerd Hoffmann wrote:
>      qemu_mutex_lock(&ssd->lock);
> +    if (c) {
> +        cursor_get(c);
> +    }
> +    cursor_put(ssd->cursor);
> +    ssd->cursor = c;
>      ssd->hot_x = c->hot_x;
>      ssd->hot_y = c->hot_y;

Coverity complains that this would dereference a NULL c, and I think
it's right; either an unlock+return is missing, or the "if" is unnecessary.

Paolo
Marc-André Lureau Feb. 25, 2016, 11:23 p.m. UTC | #2 
Hi

On Fri, Feb 26, 2016 at 12:10 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 23/02/2016 15:24, Gerd Hoffmann wrote:
>>      qemu_mutex_lock(&ssd->lock);
>> +    if (c) {
>> +        cursor_get(c);
>> +    }
>> +    cursor_put(ssd->cursor);
>> +    ssd->cursor = c;
>>      ssd->hot_x = c->hot_x;
>>      ssd->hot_y = c->hot_y;
>
> Coverity complains that this would dereference a NULL c, and I think
> it's right; either an unlock+return is missing, or the "if" is unnecessary.
>

Oops, I don't know why there is a if there, it's unnecessary indeed.
ACK if you did the patch already ;)
Paolo Bonzini Feb. 25, 2016, 11:23 p.m. UTC | #3 
On 26/02/2016 00:23, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Feb 26, 2016 at 12:10 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> On 23/02/2016 15:24, Gerd Hoffmann wrote:
>>>      qemu_mutex_lock(&ssd->lock);
>>> +    if (c) {
>>> +        cursor_get(c);
>>> +    }
>>> +    cursor_put(ssd->cursor);
>>> +    ssd->cursor = c;
>>>      ssd->hot_x = c->hot_x;
>>>      ssd->hot_y = c->hot_y;
>>
>> Coverity complains that this would dereference a NULL c, and I think
>> it's right; either an unlock+return is missing, or the "if" is unnecessary.
>>
> 
> Oops, I don't know why there is a if there, it's unnecessary indeed.
> ACK if you did the patch already ;)

No I didn't. :)

Paolo
diff mbox

Patch

diff --git a/ui/spice-display.c b/ui/spice-display.c
index cdbc78d..4e5c8a2 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -460,6 +460,13 @@  void qemu_spice_display_switch(SimpleSpiceDisplay *ssd,
 
     memset(&ssd->dirty, 0, sizeof(ssd->dirty));
     ssd->notify++;
+
+    qemu_mutex_lock(&ssd->lock);
+    if (ssd->cursor) {
+        g_free(ssd->ptr_define);
+        ssd->ptr_define = qemu_spice_create_cursor_update(ssd, ssd->cursor, 0);
+    }
+    qemu_mutex_unlock(&ssd->lock);
 }
 
 static void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd)
@@ -467,8 +474,6 @@  static void qemu_spice_cursor_refresh_unlocked(SimpleSpiceDisplay *ssd)
     if (ssd->cursor) {
         assert(ssd->dcl.con);
         dpy_cursor_define(ssd->dcl.con, ssd->cursor);
-        cursor_put(ssd->cursor);
-        ssd->cursor = NULL;
     }
     if (ssd->mouse_x != -1 && ssd->mouse_y != -1) {
         assert(ssd->dcl.con);
@@ -750,6 +755,11 @@  static void display_mouse_define(DisplayChangeListener *dcl,
     SimpleSpiceDisplay *ssd = container_of(dcl, SimpleSpiceDisplay, dcl);
 
     qemu_mutex_lock(&ssd->lock);
+    if (c) {
+        cursor_get(c);
+    }
+    cursor_put(ssd->cursor);
+    ssd->cursor = c;
     ssd->hot_x = c->hot_x;
     ssd->hot_y = c->hot_y;
     g_free(ssd->ptr_move);