Hi On Mon, Jan 15, 2024 at 2:45 PM Fiona Ebner <f.eb...@proxmox.com> wrote: > > Am 14.01.24 um 14:51 schrieb Marc-André Lureau: > >> > >> diff --git a/ui/clipboard.c b/ui/clipboard.c > >> index 3d14bffaf8..c13b54d2e9 100644 > >> --- a/ui/clipboard.c > >> +++ b/ui/clipboard.c > >> @@ -129,7 +129,8 @@ void qemu_clipboard_request(QemuClipboardInfo *info, > >> if (info->types[type].data || > >> info->types[type].requested || > >> !info->types[type].available || > >> - !info->owner) > >> + !info->owner || > >> + !info->owner->request) > >> return; > > > > While that fixes the crash, I think we should handle the situation > > earlier. A clipboard peer shouldn't be allowed to hold the clipboard > > if it doesn't have the data available or a "request" callback set. > > > > Where should initialization of the cbpeer happen so that we are > guaranteed to do it also for clients that do not set the > VNC_FEATURE_CLIPBOARD_EXT feature? Can the vnc_clipboard_request > function be re-used for clients without that feature or will it be > necessary to add some kind of "dummy" request callback for those clients?
qemu_clipboard_update() shouldn't accept info as current clipboard if the owner doesn't have the data available or a "request" callback set. This should also be assert() somehow and handled earlier. In vnc_client_cut_text_ext() we could detect that situation, but with Daniel's "[PATCH] ui: reject extended clipboard message if not activated", this shouldn't happen anymore iiuc. > > > Iow, we should have an assert(info->owner->request != NULL) here instead. > > > Your choice of course, but it would be a crash again should the issue > ever re-appear. Would error message (so the issue gets noticed) + return > be an option too? > > Best Regards, > Fiona > -- Marc-André Lureau
