[PATCH v2 00/13] Clipboard fixes (for 6.1?)

[PATCH v2 00/13] Clipboard fixes (for 6.1?)

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi,

Here is a few fixes I have collected while working on clipboard-related code.

There are some obvious code improvements/fixes, and better handling of release &
unregister to avoid dangling pointers and improve user experience.

v2:
 - replaced "ui/vdagent: unregister clipboard peer on finalize" with "ui/vdagent: disconnect handlers and reset state on finalize" patch.
 - added "ui/vdagent: reset outbuf on disconnect"
 - commit message tweaks

Marc-André Lureau (13):
  ui/vdagent: fix leak on error path
  ui/vdagent: remove copy-pasta comment
  ui/gtk-clipboard: use existing macros
  ui/gtk-clipboard: fix clipboard enum typo
  ui/clipboard: add helper to retrieve current clipboard
  ui/clipboard: release owned grabs on unregister
  ui/vdagent: disconnect handlers and reset state on finalize
  ui/vdagent: reset outbuf on disconnect
  ui/vdagent: split clipboard recv message handling
  ui/vdagent: use qemu_clipboard_info helper
  ui/gtk-clipboard: use qemu_clipboard_info helper
  ui/vdagent: send release when no clipboard owner
  ui/gtk-clipboard: emit release clipboard events

 include/ui/clipboard.h |  11 ++
 include/ui/gtk.h       |   1 -
 ui/clipboard.c         |  24 +++++
 ui/gtk-clipboard.c     |  30 +++---
 ui/vdagent.c           | 225 +++++++++++++++++++++++++----------------
 5 files changed, 188 insertions(+), 103 deletions(-)

-- 
2.32.0.264.g75ae10bc75

[PATCH v2 01/13] ui/vdagent: fix leak on error path

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

"info" was leaked when more than 10 entries.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/clipboard.h | 2 ++
 ui/vdagent.c           | 4 +---
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/ui/clipboard.h b/include/ui/clipboard.h
index b45b984c9f..eb789a285a 100644
--- a/include/ui/clipboard.h
+++ b/include/ui/clipboard.h
@@ -190,4 +190,6 @@ void qemu_clipboard_set_data(QemuClipboardPeer *peer,
                              const void *data,
                              bool update);
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QemuClipboardInfo, qemu_clipboard_info_unref)
+
 #endif /* QEMU_CLIPBOARD_H */
diff --git a/ui/vdagent.c b/ui/vdagent.c
index a253a8fe63..f6ef8d1993 100644
--- a/ui/vdagent.c
+++ b/ui/vdagent.c
@@ -438,7 +438,7 @@ static void vdagent_chr_recv_clipboard(VDAgentChardev *vd, VDAgentMessage *msg)
     uint8_t s = VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD;
     uint32_t size = msg->size;
     void *data = msg->data;
-    QemuClipboardInfo *info;
+    g_autoptr(QemuClipboardInfo) info = NULL;
     QemuClipboardType type;
 
     if (have_selection(vd)) {
@@ -477,7 +477,6 @@ static void vdagent_chr_recv_clipboard(VDAgentChardev *vd, VDAgentMessage *msg)
             size -= sizeof(uint32_t);
         }
         qemu_clipboard_update(info);
-        qemu_clipboard_info_unref(info);
         break;
     case VD_AGENT_CLIPBOARD_REQUEST:
         if (size < sizeof(uint32_t)) {
@@ -523,7 +522,6 @@ static void vdagent_chr_recv_clipboard(VDAgentChardev *vd, VDAgentMessage *msg)
             /* set empty clipboard info */
             info = qemu_clipboard_info_new(NULL, s);
             qemu_clipboard_update(info);
-            qemu_clipboard_info_unref(info);
         }
         break;
     }
-- 
2.32.0.264.g75ae10bc75

[PATCH v2 02/13] ui/vdagent: remove copy-pasta comment

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/vdagent.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/vdagent.c b/ui/vdagent.c
index f6ef8d1993..5ae5734c81 100644
--- a/ui/vdagent.c
+++ b/ui/vdagent.c
@@ -516,7 +516,7 @@ static void vdagent_chr_recv_clipboard(VDAgentChardev *vd, VDAgentMessage *msg)
         qemu_clipboard_set_data(&vd->cbpeer, vd->cbinfo[s], type,
                                 size, data, true);
         break;
-    case VD_AGENT_CLIPBOARD_RELEASE: /* data */
+    case VD_AGENT_CLIPBOARD_RELEASE:
         if (vd->cbinfo[s] &&
             vd->cbinfo[s]->owner == &vd->cbpeer) {
             /* set empty clipboard info */
-- 
2.32.0.264.g75ae10bc75

[PATCH v2 03/13] ui/gtk-clipboard: use existing macros

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hardcoding strings is error prone, use dedicated macros instead.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/gtk-clipboard.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ui/gtk-clipboard.c b/ui/gtk-clipboard.c
index bff28d2030..5e817ae55c 100644
--- a/ui/gtk-clipboard.c
+++ b/ui/gtk-clipboard.c
@@ -177,11 +177,11 @@ void gd_clipboard_init(GtkDisplayState *gd)
     qemu_clipboard_peer_register(&gd->cbpeer);
 
     gd->gtkcb[QEMU_CLIPBOARD_SELECTION_CLIPBOARD] =
-        gtk_clipboard_get(gdk_atom_intern("CLIPBOARD", FALSE));
+        gtk_clipboard_get(GDK_SELECTION_CLIPBOARD);
     gd->gtkcb[QEMU_CLIPBOARD_SELECTION_PRIMARY] =
-        gtk_clipboard_get(gdk_atom_intern("PRIMARY", FALSE));
+        gtk_clipboard_get(GDK_SELECTION_PRIMARY);
     gd->gtkcb[QEMU_CLIPBOARD_SELECTION_SECONDARY] =
-        gtk_clipboard_get(gdk_atom_intern("SECONDARY", FALSE));
+        gtk_clipboard_get(GDK_SELECTION_SECONDARY);
 
     g_signal_connect(gd->gtkcb[QEMU_CLIPBOARD_SELECTION_CLIPBOARD],
                      "owner-change", G_CALLBACK(gd_owner_change), gd);
-- 
2.32.0.264.g75ae10bc75

[PATCH v2 04/13] ui/gtk-clipboard: fix clipboard enum typo

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/gtk-clipboard.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ui/gtk-clipboard.c b/ui/gtk-clipboard.c
index 5e817ae55c..2c78de9500 100644
--- a/ui/gtk-clipboard.c
+++ b/ui/gtk-clipboard.c
@@ -155,7 +155,7 @@ static void gd_owner_change(GtkClipboard *clipboard,
 
 
     switch (event->owner_change.reason) {
-    case GDK_SETTING_ACTION_NEW:
+    case GDK_OWNER_CHANGE_NEW_OWNER:
         info = qemu_clipboard_info_new(&gd->cbpeer, s);
         if (gtk_clipboard_wait_is_text_available(clipboard)) {
             info->types[QEMU_CLIPBOARD_TYPE_TEXT].available = true;
-- 
2.32.0.264.g75ae10bc75

[PATCH v2 05/13] ui/clipboard: add helper to retrieve current clipboard

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/clipboard.h |  9 +++++++++
 ui/clipboard.c         | 15 +++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/ui/clipboard.h b/include/ui/clipboard.h
index eb789a285a..e9fcb15c66 100644
--- a/include/ui/clipboard.h
+++ b/include/ui/clipboard.h
@@ -109,6 +109,15 @@ void qemu_clipboard_peer_register(QemuClipboardPeer *peer);
  */
 void qemu_clipboard_peer_unregister(QemuClipboardPeer *peer);
 
+/**
+ * qemu_clipboard_info
+ *
+ * @selection: clipboard selection.
+ *
+ * Return the current clipboard data & owner informations.
+ */
+QemuClipboardInfo *qemu_clipboard_info(QemuClipboardSelection selection);
+
 /**
  * qemu_clipboard_info_new
  *
diff --git a/ui/clipboard.c b/ui/clipboard.c
index 3525b30178..56c14509fe 100644
--- a/ui/clipboard.c
+++ b/ui/clipboard.c
@@ -4,6 +4,8 @@
 static NotifierList clipboard_notifiers =
     NOTIFIER_LIST_INITIALIZER(clipboard_notifiers);
 
+static QemuClipboardInfo *cbinfo[QEMU_CLIPBOARD_SELECTION__COUNT];
+
 void qemu_clipboard_peer_register(QemuClipboardPeer *peer)
 {
     notifier_list_add(&clipboard_notifiers, &peer->update);
@@ -16,7 +18,20 @@ void qemu_clipboard_peer_unregister(QemuClipboardPeer *peer)
 
 void qemu_clipboard_update(QemuClipboardInfo *info)
 {
+    g_autoptr(QemuClipboardInfo) old = NULL;
+    assert(info->selection < QEMU_CLIPBOARD_SELECTION__COUNT);
+
     notifier_list_notify(&clipboard_notifiers, info);
+
+    old = cbinfo[info->selection];
+    cbinfo[info->selection] = qemu_clipboard_info_ref(info);
+}
+
+QemuClipboardInfo *qemu_clipboard_info(QemuClipboardSelection selection)
+{
+    assert(selection < QEMU_CLIPBOARD_SELECTION__COUNT);
+
+    return cbinfo[selection];
 }
 
 QemuClipboardInfo *qemu_clipboard_info_new(QemuClipboardPeer *owner,
-- 
2.32.0.264.g75ae10bc75

[PATCH v2 06/13] ui/clipboard: release owned grabs on unregister

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/clipboard.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/ui/clipboard.c b/ui/clipboard.c
index 56c14509fe..a9512f01a7 100644
--- a/ui/clipboard.c
+++ b/ui/clipboard.c
@@ -13,6 +13,15 @@ void qemu_clipboard_peer_register(QemuClipboardPeer *peer)
 
 void qemu_clipboard_peer_unregister(QemuClipboardPeer *peer)
 {
+    int i;
+
+    for (i = 0; i < QEMU_CLIPBOARD_SELECTION__COUNT; i++) {
+        if (cbinfo[i] && cbinfo[i]->owner == peer) {
+            /* release owned grabs */
+            g_autoptr(QemuClipboardInfo) info = qemu_clipboard_info_new(NULL, i);
+            qemu_clipboard_update(info);
+        }
+    }
     notifier_remove(&peer->update);
 }
 
-- 
2.32.0.264.g75ae10bc75

[PATCH v2 07/13] ui/vdagent: disconnect handlers and reset state on finalize

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Avoid handlers being called with dangling pointers when the object is
freed.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/vdagent.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/ui/vdagent.c b/ui/vdagent.c
index 5ae5734c81..1a29016e07 100644
--- a/ui/vdagent.c
+++ b/ui/vdagent.c
@@ -721,22 +721,26 @@ static void vdagent_chr_accept_input(Chardev *chr)
     vdagent_send_buf(vd);
 }
 
+static void vdagent_disconnect(VDAgentChardev *vd)
+{
+    vdagent_reset_bufs(vd);
+    vd->caps = 0;
+    if (vd->mouse_hs) {
+        qemu_input_handler_deactivate(vd->mouse_hs);
+    }
+    if (vd->cbpeer.update.notify) {
+        qemu_clipboard_peer_unregister(&vd->cbpeer);
+        memset(&vd->cbpeer, 0, sizeof(vd->cbpeer));
+    }
+}
+
 static void vdagent_chr_set_fe_open(struct Chardev *chr, int fe_open)
 {
     VDAgentChardev *vd = QEMU_VDAGENT_CHARDEV(chr);
 
     if (!fe_open) {
         trace_vdagent_close();
-        /* reset state */
-        vdagent_reset_bufs(vd);
-        vd->caps = 0;
-        if (vd->mouse_hs) {
-            qemu_input_handler_deactivate(vd->mouse_hs);
-        }
-        if (vd->cbpeer.update.notify) {
-            qemu_clipboard_peer_unregister(&vd->cbpeer);
-            memset(&vd->cbpeer, 0, sizeof(vd->cbpeer));
-        }
+        vdagent_disconnect(vd);
         return;
     }
 
@@ -781,6 +785,7 @@ static void vdagent_chr_fini(Object *obj)
 {
     VDAgentChardev *vd = QEMU_VDAGENT_CHARDEV(obj);
 
+    vdagent_disconnect(vd);
     buffer_free(&vd->outbuf);
 }
 
-- 
2.32.0.264.g75ae10bc75

[PATCH v2 08/13] ui/vdagent: reset outbuf on disconnect

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/vdagent.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/vdagent.c b/ui/vdagent.c
index 1a29016e07..3770c58b6c 100644
--- a/ui/vdagent.c
+++ b/ui/vdagent.c
@@ -723,6 +723,7 @@ static void vdagent_chr_accept_input(Chardev *chr)
 
 static void vdagent_disconnect(VDAgentChardev *vd)
 {
+    buffer_reset(&vd->outbuf);
     vdagent_reset_bufs(vd);
     vd->caps = 0;
     if (vd->mouse_hs) {
-- 
2.32.0.264.g75ae10bc75

[PATCH v2 09/13] ui/vdagent: split clipboard recv message handling

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/vdagent.c | 157 +++++++++++++++++++++++++++++----------------------
 1 file changed, 89 insertions(+), 68 deletions(-)

diff --git a/ui/vdagent.c b/ui/vdagent.c
index 3770c58b6c..362064213c 100644
--- a/ui/vdagent.c
+++ b/ui/vdagent.c
@@ -433,13 +433,94 @@ static void vdagent_clipboard_request(QemuClipboardInfo *info,
     vdagent_send_msg(vd, msg);
 }
 
+static void vdagent_clipboard_recv_grab(VDAgentChardev *vd, uint8_t s, uint32_t size, void *data)
+{
+    g_autoptr(QemuClipboardInfo) info = NULL;
+
+    trace_vdagent_cb_grab_selection(GET_NAME(sel_name, s));
+    info = qemu_clipboard_info_new(&vd->cbpeer, s);
+    if (size > sizeof(uint32_t) * 10) {
+        /*
+         * spice has 6 types as of 2021. Limiting to 10 entries
+         * so we we have some wiggle room.
+         */
+        return;
+    }
+    while (size >= sizeof(uint32_t)) {
+        trace_vdagent_cb_grab_type(GET_NAME(type_name, *(uint32_t *)data));
+        switch (*(uint32_t *)data) {
+        case VD_AGENT_CLIPBOARD_UTF8_TEXT:
+            info->types[QEMU_CLIPBOARD_TYPE_TEXT].available = true;
+            break;
+        default:
+            break;
+        }
+        data += sizeof(uint32_t);
+        size -= sizeof(uint32_t);
+    }
+    qemu_clipboard_update(info);
+}
+
+static void vdagent_clipboard_recv_request(VDAgentChardev *vd, uint8_t s, uint32_t size, void *data)
+{
+    QemuClipboardType type;
+
+    if (size < sizeof(uint32_t)) {
+        return;
+    }
+    switch (*(uint32_t *)data) {
+    case VD_AGENT_CLIPBOARD_UTF8_TEXT:
+        type = QEMU_CLIPBOARD_TYPE_TEXT;
+        break;
+    default:
+        return;
+    }
+    if (vd->cbinfo[s] && vd->cbinfo[s]->types[type].available &&
+        vd->cbinfo[s]->owner != &vd->cbpeer) {
+        if (vd->cbinfo[s]->types[type].data) {
+            vdagent_send_clipboard_data(vd, vd->cbinfo[s], type);
+        } else {
+            vd->cbpending[s] |= (1 << type);
+            qemu_clipboard_request(vd->cbinfo[s], type);
+        }
+    }
+}
+
+static void vdagent_clipboard_recv_data(VDAgentChardev *vd, uint8_t s, uint32_t size, void *data)
+{
+    QemuClipboardType type;
+
+    if (size < sizeof(uint32_t)) {
+        return;
+    }
+    switch (*(uint32_t *)data) {
+    case VD_AGENT_CLIPBOARD_UTF8_TEXT:
+        type = QEMU_CLIPBOARD_TYPE_TEXT;
+        break;
+    default:
+        return;
+    }
+    data += 4;
+    size -= 4;
+    qemu_clipboard_set_data(&vd->cbpeer, vd->cbinfo[s], type, size, data, true);
+}
+
+static void vdagent_clipboard_recv_release(VDAgentChardev *vd, uint8_t s)
+{
+    g_autoptr(QemuClipboardInfo) info = NULL;
+
+    if (vd->cbinfo[s] && vd->cbinfo[s]->owner == &vd->cbpeer) {
+        /* set empty clipboard info */
+        info = qemu_clipboard_info_new(NULL, s);
+        qemu_clipboard_update(info);
+    }
+}
+
 static void vdagent_chr_recv_clipboard(VDAgentChardev *vd, VDAgentMessage *msg)
 {
     uint8_t s = VD_AGENT_CLIPBOARD_SELECTION_CLIPBOARD;
     uint32_t size = msg->size;
     void *data = msg->data;
-    g_autoptr(QemuClipboardInfo) info = NULL;
-    QemuClipboardType type;
 
     if (have_selection(vd)) {
         if (size < 4) {
@@ -455,75 +536,15 @@ static void vdagent_chr_recv_clipboard(VDAgentChardev *vd, VDAgentMessage *msg)
 
     switch (msg->type) {
     case VD_AGENT_CLIPBOARD_GRAB:
-        trace_vdagent_cb_grab_selection(GET_NAME(sel_name, s));
-        info = qemu_clipboard_info_new(&vd->cbpeer, s);
-        if (size > sizeof(uint32_t) * 10) {
-            /*
-             * spice has 6 types as of 2021. Limiting to 10 entries
-             * so we we have some wiggle room.
-             */
-            return;
-        }
-        while (size >= sizeof(uint32_t)) {
-            trace_vdagent_cb_grab_type(GET_NAME(type_name, *(uint32_t *)data));
-            switch (*(uint32_t *)data) {
-            case VD_AGENT_CLIPBOARD_UTF8_TEXT:
-                info->types[QEMU_CLIPBOARD_TYPE_TEXT].available = true;
-                break;
-            default:
-                break;
-            }
-            data += sizeof(uint32_t);
-            size -= sizeof(uint32_t);
-        }
-        qemu_clipboard_update(info);
-        break;
+        return vdagent_clipboard_recv_grab(vd, s, size, data);
     case VD_AGENT_CLIPBOARD_REQUEST:
-        if (size < sizeof(uint32_t)) {
-            return;
-        }
-        switch (*(uint32_t *)data) {
-        case VD_AGENT_CLIPBOARD_UTF8_TEXT:
-            type = QEMU_CLIPBOARD_TYPE_TEXT;
-            break;
-        default:
-            return;
-        }
-        if (vd->cbinfo[s] &&
-            vd->cbinfo[s]->types[type].available &&
-            vd->cbinfo[s]->owner != &vd->cbpeer) {
-            if (vd->cbinfo[s]->types[type].data) {
-                vdagent_send_clipboard_data(vd, vd->cbinfo[s], type);
-            } else {
-                vd->cbpending[s] |= (1 << type);
-                qemu_clipboard_request(vd->cbinfo[s], type);
-            }
-        }
-        break;
+        return vdagent_clipboard_recv_request(vd, s, size, data);
     case VD_AGENT_CLIPBOARD: /* data */
-        if (size < sizeof(uint32_t)) {
-            return;
-        }
-        switch (*(uint32_t *)data) {
-        case VD_AGENT_CLIPBOARD_UTF8_TEXT:
-            type = QEMU_CLIPBOARD_TYPE_TEXT;
-            break;
-        default:
-            return;
-        }
-        data += 4;
-        size -= 4;
-        qemu_clipboard_set_data(&vd->cbpeer, vd->cbinfo[s], type,
-                                size, data, true);
-        break;
+        return vdagent_clipboard_recv_data(vd, s, size, data);
     case VD_AGENT_CLIPBOARD_RELEASE:
-        if (vd->cbinfo[s] &&
-            vd->cbinfo[s]->owner == &vd->cbpeer) {
-            /* set empty clipboard info */
-            info = qemu_clipboard_info_new(NULL, s);
-            qemu_clipboard_update(info);
-        }
-        break;
+        return vdagent_clipboard_recv_release(vd, s);
+    default:
+        g_assert_not_reached();
     }
 }
 
-- 
2.32.0.264.g75ae10bc75

[PATCH v2 10/13] ui/vdagent: use qemu_clipboard_info helper

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/vdagent.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/ui/vdagent.c b/ui/vdagent.c
index 362064213c..bc8347f995 100644
--- a/ui/vdagent.c
+++ b/ui/vdagent.c
@@ -47,7 +47,6 @@ struct VDAgentChardev {
 
     /* clipboard */
     QemuClipboardPeer cbpeer;
-    QemuClipboardInfo *cbinfo[QEMU_CLIPBOARD_SELECTION__COUNT];
     uint32_t cbpending[QEMU_CLIPBOARD_SELECTION__COUNT];
 };
 typedef struct VDAgentChardev VDAgentChardev;
@@ -384,9 +383,7 @@ static void vdagent_clipboard_notify(Notifier *notifier, void *data)
     QemuClipboardType type;
     bool self_update = info->owner == &vd->cbpeer;
 
-    if (info != vd->cbinfo[s]) {
-        qemu_clipboard_info_unref(vd->cbinfo[s]);
-        vd->cbinfo[s] = qemu_clipboard_info_ref(info);
+    if (info != qemu_clipboard_info(s)) {
         vd->cbpending[s] = 0;
         if (!self_update) {
             vdagent_send_clipboard_grab(vd, info);
@@ -464,6 +461,7 @@ static void vdagent_clipboard_recv_grab(VDAgentChardev *vd, uint8_t s, uint32_t
 static void vdagent_clipboard_recv_request(VDAgentChardev *vd, uint8_t s, uint32_t size, void *data)
 {
     QemuClipboardType type;
+    QemuClipboardInfo *info;
 
     if (size < sizeof(uint32_t)) {
         return;
@@ -475,13 +473,14 @@ static void vdagent_clipboard_recv_request(VDAgentChardev *vd, uint8_t s, uint32
     default:
         return;
     }
-    if (vd->cbinfo[s] && vd->cbinfo[s]->types[type].available &&
-        vd->cbinfo[s]->owner != &vd->cbpeer) {
-        if (vd->cbinfo[s]->types[type].data) {
-            vdagent_send_clipboard_data(vd, vd->cbinfo[s], type);
+
+    info = qemu_clipboard_info(s);
+    if (info && info->types[type].available && info->owner != &vd->cbpeer) {
+        if (info->types[type].data) {
+            vdagent_send_clipboard_data(vd, info, type);
         } else {
             vd->cbpending[s] |= (1 << type);
-            qemu_clipboard_request(vd->cbinfo[s], type);
+            qemu_clipboard_request(info, type);
         }
     }
 }
@@ -489,6 +488,7 @@ static void vdagent_clipboard_recv_request(VDAgentChardev *vd, uint8_t s, uint32
 static void vdagent_clipboard_recv_data(VDAgentChardev *vd, uint8_t s, uint32_t size, void *data)
 {
     QemuClipboardType type;
+    QemuClipboardInfo *info;
 
     if (size < sizeof(uint32_t)) {
         return;
@@ -502,14 +502,20 @@ static void vdagent_clipboard_recv_data(VDAgentChardev *vd, uint8_t s, uint32_t
     }
     data += 4;
     size -= 4;
-    qemu_clipboard_set_data(&vd->cbpeer, vd->cbinfo[s], type, size, data, true);
+
+    info = qemu_clipboard_info(s);
+    if (info->owner == &vd->cbpeer) {
+        qemu_clipboard_set_data(&vd->cbpeer, info, type, size, data, true);
+    }
 }
 
 static void vdagent_clipboard_recv_release(VDAgentChardev *vd, uint8_t s)
 {
+    QemuClipboardInfo *cur;
     g_autoptr(QemuClipboardInfo) info = NULL;
 
-    if (vd->cbinfo[s] && vd->cbinfo[s]->owner == &vd->cbpeer) {
+    cur = qemu_clipboard_info(s);
+    if (cur && cur->owner == &vd->cbpeer) {
         /* set empty clipboard info */
         info = qemu_clipboard_info_new(NULL, s);
         qemu_clipboard_update(info);
-- 
2.32.0.264.g75ae10bc75

[PATCH v2 11/13] ui/gtk-clipboard: use qemu_clipboard_info helper

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/gtk.h   |  1 -
 ui/gtk-clipboard.c | 14 ++++++--------
 2 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/include/ui/gtk.h b/include/ui/gtk.h
index 9516670ebc..60e9cdc39c 100644
--- a/include/ui/gtk.h
+++ b/include/ui/gtk.h
@@ -139,7 +139,6 @@ struct GtkDisplayState {
     bool external_pause_update;
 
     QemuClipboardPeer cbpeer;
-    QemuClipboardInfo *cbinfo[QEMU_CLIPBOARD_SELECTION__COUNT];
     uint32_t cbpending[QEMU_CLIPBOARD_SELECTION__COUNT];
     GtkClipboard *gtkcb[QEMU_CLIPBOARD_SELECTION__COUNT];
     bool cbowner[QEMU_CLIPBOARD_SELECTION__COUNT];
diff --git a/ui/gtk-clipboard.c b/ui/gtk-clipboard.c
index 2c78de9500..4e4b3c52bb 100644
--- a/ui/gtk-clipboard.c
+++ b/ui/gtk-clipboard.c
@@ -45,24 +45,24 @@ static void gd_clipboard_get_data(GtkClipboard     *clipboard,
     GtkDisplayState *gd = data;
     QemuClipboardSelection s = gd_find_selection(gd, clipboard);
     QemuClipboardType type = QEMU_CLIPBOARD_TYPE_TEXT;
-    QemuClipboardInfo *info = qemu_clipboard_info_ref(gd->cbinfo[s]);
+    g_autoptr(QemuClipboardInfo) info = NULL;
+
+    info = qemu_clipboard_info_ref(qemu_clipboard_info(s));
 
     qemu_clipboard_request(info, type);
-    while (info == gd->cbinfo[s] &&
+    while (info == qemu_clipboard_info(s) &&
            info->types[type].available &&
            info->types[type].data == NULL) {
         main_loop_wait(false);
     }
 
-    if (info == gd->cbinfo[s] && gd->cbowner[s]) {
+    if (info == qemu_clipboard_info(s) && gd->cbowner[s]) {
         gtk_selection_data_set_text(selection_data,
                                     info->types[type].data,
                                     info->types[type].size);
     } else {
         /* clipboard owner changed while waiting for the data */
     }
-
-    qemu_clipboard_info_unref(info);
 }
 
 static void gd_clipboard_clear(GtkClipboard *clipboard,
@@ -81,9 +81,7 @@ static void gd_clipboard_notify(Notifier *notifier, void *data)
     QemuClipboardSelection s = info->selection;
     bool self_update = info->owner == &gd->cbpeer;
 
-    if (info != gd->cbinfo[s]) {
-        qemu_clipboard_info_unref(gd->cbinfo[s]);
-        gd->cbinfo[s] = qemu_clipboard_info_ref(info);
+    if (info != qemu_clipboard_info(s)) {
         gd->cbpending[s] = 0;
         if (!self_update) {
             GtkTargetList *list;
-- 
2.32.0.264.g75ae10bc75

[PATCH v2 12/13] ui/vdagent: send release when no clipboard owner

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/vdagent.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/ui/vdagent.c b/ui/vdagent.c
index bc8347f995..efb4546045 100644
--- a/ui/vdagent.c
+++ b/ui/vdagent.c
@@ -345,6 +345,24 @@ static void vdagent_send_clipboard_grab(VDAgentChardev *vd,
     vdagent_send_msg(vd, msg);
 }
 
+static void vdagent_send_clipboard_release(VDAgentChardev *vd,
+                                           QemuClipboardInfo *info)
+{
+    g_autofree VDAgentMessage *msg = g_malloc0(sizeof(VDAgentMessage) +
+                                               sizeof(uint32_t));
+
+    if (have_selection(vd)) {
+        uint8_t *s = msg->data;
+        *s = info->selection;
+        msg->size += sizeof(uint32_t);
+    } else if (info->selection != QEMU_CLIPBOARD_SELECTION_CLIPBOARD) {
+        return;
+    }
+
+    msg->type = VD_AGENT_CLIPBOARD_RELEASE;
+    vdagent_send_msg(vd, msg);
+}
+
 static void vdagent_send_clipboard_data(VDAgentChardev *vd,
                                         QemuClipboardInfo *info,
                                         QemuClipboardType type)
@@ -386,7 +404,11 @@ static void vdagent_clipboard_notify(Notifier *notifier, void *data)
     if (info != qemu_clipboard_info(s)) {
         vd->cbpending[s] = 0;
         if (!self_update) {
-            vdagent_send_clipboard_grab(vd, info);
+            if (info->owner) {
+                vdagent_send_clipboard_grab(vd, info);
+            } else {
+                vdagent_send_clipboard_release(vd, info);
+            }
         }
         return;
     }
-- 
2.32.0.264.g75ae10bc75

[PATCH v2 13/13] ui/gtk-clipboard: emit release clipboard events

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/gtk-clipboard.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ui/gtk-clipboard.c b/ui/gtk-clipboard.c
index 4e4b3c52bb..16b2e2063e 100644
--- a/ui/gtk-clipboard.c
+++ b/ui/gtk-clipboard.c
@@ -144,7 +144,7 @@ static void gd_owner_change(GtkClipboard *clipboard,
 {
     GtkDisplayState *gd = data;
     QemuClipboardSelection s = gd_find_selection(gd, clipboard);
-    QemuClipboardInfo *info;
+    g_autoptr(QemuClipboardInfo) info = NULL;
 
     if (gd->cbowner[s]) {
         /* ignore notifications about our own grabs */
@@ -158,13 +158,13 @@ static void gd_owner_change(GtkClipboard *clipboard,
         if (gtk_clipboard_wait_is_text_available(clipboard)) {
             info->types[QEMU_CLIPBOARD_TYPE_TEXT].available = true;
         }
-
-        qemu_clipboard_update(info);
-        qemu_clipboard_info_unref(info);
         break;
     default:
+        info = qemu_clipboard_info_new(NULL, s);
+        gd->cbowner[s] = false;
         break;
     }
+    qemu_clipboard_update(info);
 }
 
 void gd_clipboard_init(GtkDisplayState *gd)
-- 
2.32.0.264.g75ae10bc75

Re: [PATCH v2 00/13] Clipboard fixes (for 6.1?)

[Marc-André Lureau]

[-- Attachment #1: Type: text/plain, Size: 1915 bytes --]

Hi Gerd,

On Wed, Jul 21, 2021 at 12:41 PM <marcandre.lureau@redhat.com> wrote:

> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi,
>
> Here is a few fixes I have collected while working on clipboard-related
> code.
>
> There are some obvious code improvements/fixes, and better handling of
> release &
> unregister to avoid dangling pointers and improve user experience.
>
> v2:
>  - replaced "ui/vdagent: unregister clipboard peer on finalize" with
> "ui/vdagent: disconnect handlers and reset state on finalize" patch.
>  - added "ui/vdagent: reset outbuf on disconnect"
>  - commit message tweaks
>
>
I am still improving this series, but any feedback is welcome.

I am wondering if we should add a migration blocker for 6.1. VNC clients
are reconnected on migration, but migration shouldn't be visible to guest
agent. What do you think?

thanks



> Marc-André Lureau (13):
>   ui/vdagent: fix leak on error path
>   ui/vdagent: remove copy-pasta comment
>   ui/gtk-clipboard: use existing macros
>   ui/gtk-clipboard: fix clipboard enum typo
>   ui/clipboard: add helper to retrieve current clipboard
>   ui/clipboard: release owned grabs on unregister
>   ui/vdagent: disconnect handlers and reset state on finalize
>   ui/vdagent: reset outbuf on disconnect
>   ui/vdagent: split clipboard recv message handling
>   ui/vdagent: use qemu_clipboard_info helper
>   ui/gtk-clipboard: use qemu_clipboard_info helper
>   ui/vdagent: send release when no clipboard owner
>   ui/gtk-clipboard: emit release clipboard events
>
>  include/ui/clipboard.h |  11 ++
>  include/ui/gtk.h       |   1 -
>  ui/clipboard.c         |  24 +++++
>  ui/gtk-clipboard.c     |  30 +++---
>  ui/vdagent.c           | 225 +++++++++++++++++++++++++----------------
>  5 files changed, 188 insertions(+), 103 deletions(-)
>
> --
> 2.32.0.264.g75ae10bc75
>
>
>

[-- Attachment #2: Type: text/html, Size: 2674 bytes --]