Hi

On Wed, Jul 30, 2025 at 4:23 PM startergo <starte...@protonmail.com> wrote:
>
> From: startergo <starte...@protonmail.com>
> Date: Wed, 30 Jul 2025 12:13:25 +0000
> Subject: [PATCH] ui/sdl2: Add clipboard support with async handling
>
> This patch adds clipboard support to the SDL2 UI backend with proper
> asynchronous clipboard request handling and QEMU clipboard subsystem
> integration.
>
> Key features:
> - Runtime stability: QEMU starts and runs without crashes
> - Async handling: Proper async clipboard request tracking
> - Error handling: Comprehensive SDL error reporting
> - Memory management: Correct use of g_autofree and proper cleanup
> - QEMU integration: Full integration with QEMU's clipboard subsystem
>
> The implementation includes:
> - New meson build option 'sdl_clipboard' (enabled by default)
> - Proper clipboard peer registration and notification handling
> - Async request handling to prevent blocking operations
> - Memory-safe string handling with proper null termination
>
> Signed-off-by: startergo <starte...@protonmail.com>
> Co-authored-by: Kamay Xutax <ad...@xutaxkamay.com>
> ---
>  include/ui/sdl2.h     |   8 ++
>  meson.build           |   3 +
>  meson_options.txt     |   2 +
>  ui/meson.build        |   3 +
>  ui/sdl2-clipboard.c   | 154 ++++++++++++++++++++++++++++++++++++++++++
>  ui/sdl2.c             |   9 +++
>  6 files changed, 179 insertions(+)
>  create mode 100644 ui/sdl2-clipboard.c
>

It still doesn't apply:

git am \[PATCH\ RFC\ v1\ 1_1\]\ ui_sdl2_\ clipboard\ sharing\
implementation\ for\ SDL.eml
warning: quoted CRLF detected
Applying: ui/sdl2: Add clipboard support with async handling
error: corrupt patch at line 16
Patch failed at 0001 ui/sdl2: Add clipboard support with async handling


> diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
> index 1234567..abcdefg 100644
> --- a/include/ui/sdl2.h
> +++ b/include/ui/sdl2.h
> @@ -21,6 +21,10 @@
>  # include <SDL_image.h>
>  #endif
>
> +#ifdef CONFIG_SDL_CLIPBOARD
> +#include "ui/clipboard.h"
> +#endif
> +
>  #include "ui/kbd-state.h"
>  #ifdef CONFIG_OPENGL
>  # include "ui/egl-helpers.h"
> @@ -45,6 +49,9 @@ struct sdl2_console {
>      bool gui_keysym;
>      SDL_GLContext winctx;
>      QKbdState *kbd;
> +#ifdef CONFIG_SDL_CLIPBOARD
> +    QemuClipboardPeer cbpeer;
> +#endif
>  #ifdef CONFIG_OPENGL
>      QemuGLShader *gls;
>      egl_fb guest_fb;
> @@ -96,5 +103,10 @@ void sdl2_gl_scanout_dmabuf(DisplayChangeListener *dcl,
>                               void *d3d_tex2d);
>  void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
>                             uint32_t x, uint32_t y, uint32_t w, uint32_t h);
> +
> +#ifdef CONFIG_SDL_CLIPBOARD
> +void sdl2_clipboard_init(struct sdl2_console *scon);
> +void sdl2_clipboard_handle_request(struct sdl2_console *scon);
> +#endif
>
>  #endif /* SDL2_H */
> diff --git a/meson.build b/meson.build
> index 1234567..abcdefg 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1596,6 +1596,8 @@ else
>    sdl_image = not_found
>  endif
>
> +have_sdl_clipboard = sdl.found() and get_option('sdl_clipboard')
> +
>  rbd = not_found
>  if not get_option('rbd').auto() or have_block
>    librados = cc.find_library('rados', required: get_option('rbd'))
> @@ -2511,6 +2513,7 @@ config_host_data.set('CONFIG_SAFESTACK', 
> get_option('safe_stack'))
>  config_host_data.set('CONFIG_SDL', sdl.found())
>  config_host_data.set('CONFIG_SDL_IMAGE', sdl_image.found())
> +config_host_data.set('CONFIG_SDL_CLIPBOARD', have_sdl_clipboard)
>  config_host_data.set('CONFIG_SECCOMP', seccomp.found())
>  if seccomp.found()
>    config_host_data.set('CONFIG_SECCOMP_SYSRAWRC', seccomp_has_sysrawrc)
> diff --git a/meson_options.txt b/meson_options.txt
> index 1234567..abcdefg 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -212,6 +212,8 @@ option('sdl', type : 'feature', value : 'auto',
>         description: 'SDL user interface')
>  option('sdl_image', type : 'feature', value : 'auto',
>         description: 'SDL Image support for icons')
> +option('sdl_clipboard', type : 'boolean', value : true,
> +       description: 'SDL clipboard support')
>  option('seccomp', type : 'feature', value : 'auto',
>         description: 'seccomp support')
>  option('smartcard', type : 'feature', value : 'auto',
> diff --git a/ui/meson.build b/ui/meson.build
> index 1234567..abcdefg 100644
> --- a/ui/meson.build
> +++ b/ui/meson.build
> @@ -126,6 +126,9 @@ if sdl.found()
>      'sdl2-input.c',
>      'sdl2.c',
>    ))
> +  if have_sdl_clipboard
> +    sdl_ss.add(files('sdl2-clipboard.c'))
> +  endif
>    sdl_ss.add(when: opengl, if_true: files('sdl2-gl.c'))
>    sdl_ss.add(when: x11, if_true: files('x_keymap.c'))
>    ui_modules += {'sdl' : sdl_ss}
> diff --git a/ui/sdl2-clipboard.c b/ui/sdl2-clipboard.c
> new file mode 100644
> index 0000000..1234567
> --- /dev/null
> +++ b/ui/sdl2-clipboard.c
> @@ -0,0 +1,154 @@
> +/*
> + * SDL UI -- clipboard support (improved async version)
> + *
> + * Copyright (C) 2023 Kamay Xutax <ad...@xutaxkamay.com>
> + * Copyright (C) 2025 startergo <starte...@protonmail.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "ui/console.h"
> +#include "ui/clipboard.h"
> +#include "ui/sdl2.h"
> +#include "qemu/log.h"
> +
> +#ifdef CONFIG_SDL_CLIPBOARD
> +
> +/* Track pending clipboard requests to handle async data */
> +typedef struct {
> +    struct sdl2_console *scon;
> +    QemuClipboardInfo *info;
> +    QemuClipboardType type;
> +} SDLClipboardRequest;
> +
> +static SDLClipboardRequest *pending_request = NULL;
> +
> +static void sdl2_clipboard_clear_pending(void)
> +{
> +    if (pending_request) {
> +        if (pending_request->info) {
> +            qemu_clipboard_info_unref(pending_request->info);
> +        }
> +        g_free(pending_request);
> +        pending_request = NULL;
> +    }
> +}
> +
> +static void sdl2_clipboard_notify(Notifier *notifier, void *data)
> +{
> +    QemuClipboardNotify *notify = data;
> +    struct sdl2_console *scon =
> +        container_of(notifier, struct sdl2_console, cbpeer.notifier);
> +    bool self_update = notify->info->owner == &scon->cbpeer;
> +    const char *text_data;
> +    size_t text_size;
> +
> +    switch (notify->type) {
> +    case QEMU_CLIPBOARD_UPDATE_INFO:
> +        {
> +            /* Skip self-updates to avoid clipboard manager conflicts */
> +            if (self_update) {
> +                return;
> +            }
> +
> +            if (!notify->info->types[QEMU_CLIPBOARD_TYPE_TEXT].available) {
> +                return;
> +            }
> +
> +            /* Check if this is completion of our pending request */
> +            if (pending_request && pending_request->info == notify->info &&
> +                pending_request->type == QEMU_CLIPBOARD_TYPE_TEXT) {
> +                sdl2_clipboard_clear_pending();
> +            }
> +
> +            /* Check if data is available, request asynchronously if not */
> +            if (!notify->info->types[QEMU_CLIPBOARD_TYPE_TEXT].data) {
> +                if (!pending_request) {
> +                    pending_request = g_new0(SDLClipboardRequest, 1);
> +                    pending_request->scon = scon;
> +                    pending_request->info = 
> qemu_clipboard_info_ref(notify->info);
> +                    pending_request->type = QEMU_CLIPBOARD_TYPE_TEXT;
> +                    qemu_clipboard_request(notify->info, 
> QEMU_CLIPBOARD_TYPE_TEXT);
> +                }
> +                return;
> +            }
> +
> +            /* Process available data */
> +            text_size = notify->info->types[QEMU_CLIPBOARD_TYPE_TEXT].size;
> +            if (text_size == 0) {
> +                return;
> +            }
> +
> +            text_data = (const char 
> *)notify->info->types[QEMU_CLIPBOARD_TYPE_TEXT].data;
> +
> +            /* Ensure null termination for SDL clipboard */
> +            g_autofree char *text = g_strndup(text_data, text_size);
> +            if (text && text[0] != '\0') {
> +                SDL_SetClipboardText(text);
> +            } else if (!text) {
> +                qemu_log_mask(LOG_GUEST_ERROR,
> +                              "SDL clipboard: Failed to allocate memory for 
> clipboard text\n");
> +            }
> +            break;
> +        }
> +    case QEMU_CLIPBOARD_RESET_SERIAL:
> +        sdl2_clipboard_clear_pending();
> +        break;
> +    }
> +}
> +
> +static void sdl2_clipboard_request(QemuClipboardInfo *info,
> +                                   QemuClipboardType type)
> +{
> +    g_autofree char *text = NULL;
> +
> +    if (type != QEMU_CLIPBOARD_TYPE_TEXT) {
> +        return;
> +    }
> +
> +    text = SDL_GetClipboardText();
> +    if (!text) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "SDL clipboard: Failed to get clipboard text: %s\n",
> +                      SDL_GetError());
> +        return;
> +    }
> +
> +    qemu_clipboard_set_data(info->owner, info, type,
> +                            strlen(text), text, true);
> +}
> +
> +void sdl2_clipboard_init(struct sdl2_console *scon)
> +{
> +    scon->cbpeer.name = "sdl2-clipboard";
> +    scon->cbpeer.notifier.notify = sdl2_clipboard_notify;
> +    scon->cbpeer.request = sdl2_clipboard_request;
> +
> +    qemu_clipboard_peer_register(&scon->cbpeer);
> +}
> +
> +void sdl2_clipboard_handle_request(struct sdl2_console *scon)
> +{
> +    g_autofree char *text = NULL;
> +    QemuClipboardInfo *info;
> +
> +    text = SDL_GetClipboardText();
> +    if (!text) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "SDL clipboard: Failed to get clipboard text: %s\n",
> +                      SDL_GetError());
> +        return;
> +    }
> +
> +    if (text[0] == '\0') {
> +        return; /* Ignore empty clipboard */
> +    }
> +
> +    info = qemu_clipboard_info_new(&scon->cbpeer, 
> QEMU_CLIPBOARD_SELECTION_CLIPBOARD);
> +    qemu_clipboard_set_data(&scon->cbpeer, info, QEMU_CLIPBOARD_TYPE_TEXT,
> +                            strlen(text), text, true);
> +    qemu_clipboard_info_unref(info);
> +}
> +
> +#endif /* CONFIG_SDL_CLIPBOARD */
> diff --git a/ui/sdl2.c b/ui/sdl2.c
> index 1234567..abcdefg 100644
> --- a/ui/sdl2.c
> +++ b/ui/sdl2.c
> @@ -691,6 +691,11 @@ void sdl2_poll_events(struct sdl2_console *scon)
>          case SDL_WINDOWEVENT:
>              handle_windowevent(ev);
>              break;
> +#ifdef CONFIG_SDL_CLIPBOARD
> +        case SDL_CLIPBOARDUPDATE:
> +            sdl2_clipboard_handle_request(scon);
> +            break;
> +#endif
>          default:
>              break;
>          }
> @@ -900,6 +905,10 @@ static void sdl2_display_init(DisplayState *ds, 
> DisplayOptions *o)
>          qemu_console_set_display_gl_ctx(con, &sdl2_console[i].dgc);
>  #endif
>          register_displaychangelistener(&sdl2_console[i].dcl);
> +
> +#ifdef CONFIG_SDL_CLIPBOARD
> +        sdl2_clipboard_init(&sdl2_console[i]);
> +#endif
>
>  #if defined(SDL_VIDEO_DRIVER_WINDOWS) || defined(SDL_VIDEO_DRIVER_X11)
>          if (SDL_GetWindowWMInfo(sdl2_console[i].real_window, &info)) {
> --
> 2.34.1
>
> Sent from Proton Mail for iOS
>
>
> On Wed, Jul 30, 2025 at 13:04, Marc-André Lureau <marcandre.lur...@gmail.com> 
> wrote:
>
> Hi
>
> On Wed, Jul 30, 2025 at 1:34 PM startergo <starte...@protonmail.com> wrote:
> >
> > Hi Marc-André,
> > Please review the updated code and let me know if there is anything else 
> > left to fix:
> >
> > This update fixes:
> > ✅ Runtime Stability: QEMU starts and runs without crashes
> > ✅ Async Handling: Proper async clipboard request tracking
> > ✅ Error Handling: Comprehensive SDL error reporting
> > ✅ Memory Management: Correct use of g_autofree and proper cleanup
> > ✅ QEMU Integration: Full integration with QEMU's clipboard subsystem:
> >
>
> Please send a properly formatted git patch:
> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#do-not-send-as-an-attachment
>
> > diff -ruN qemu-10.0.0-original/include/ui/sdl2.h 
> > qemu-10.0.0-modified/include/ui/sdl2.h
> > --- qemu-10.0.0-original/include/ui/sdl2.h 2025-07-30 11:51:59
> > +++ qemu-10.0.0-modified/include/ui/sdl2.h 2025-07-30 11:58:44
> > @@ -21,6 +21,10 @@
> > # include <SDL_image.h>
> > #endif
> >
> > +#ifdef CONFIG_SDL_CLIPBOARD
> > +#include "ui/clipboard.h"
> > +#endif
> > +
> > #include "ui/kbd-state.h"
> > #ifdef CONFIG_OPENGL
> > # include "ui/egl-helpers.h"
> > @@ -45,6 +49,9 @@
> > bool gui_keysym;
> > SDL_GLContext winctx;
> > QKbdState *kbd;
> > +#ifdef CONFIG_SDL_CLIPBOARD
> > + QemuClipboardPeer cbpeer;
> > +#endif
> > #ifdef CONFIG_OPENGL
> > QemuGLShader *gls;
> > egl_fb guest_fb;
> > @@ -96,5 +103,10 @@
> > void *d3d_tex2d);
> > void sdl2_gl_scanout_flush(DisplayChangeListener *dcl,
> > uint32_t x, uint32_t y, uint32_t w, uint32_t h);
> > +
> > +#ifdef CONFIG_SDL_CLIPBOARD
> > +void sdl2_clipboard_init(struct sdl2_console *scon);
> > +void sdl2_clipboard_handle_request(struct sdl2_console *scon);
> > +#endif
> >
> > #endif /* SDL2_H */
> > diff -ruN qemu-10.0.0-original/meson.build qemu-10.0.0-modified/meson.build
> > --- qemu-10.0.0-original/meson.build 2025-07-30 11:52:13
> > +++ qemu-10.0.0-modified/meson.build 2025-07-30 11:58:28
> > @@ -1596,6 +1596,8 @@
> > sdl_image = not_found
> > endif
> >
> > +have_sdl_clipboard = sdl.found() and get_option('sdl_clipboard')
> > +
> > rbd = not_found
> > if not get_option('rbd').auto() or have_block
> > librados = cc.find_library('rados', required: get_option('rbd'))
> > @@ -2511,6 +2513,7 @@
> > config_host_data.set('CONFIG_SAFESTACK', get_option('safe_stack'))
> > config_host_data.set('CONFIG_SDL', sdl.found())
> > config_host_data.set('CONFIG_SDL_IMAGE', sdl_image.found())
> > +config_host_data.set('CONFIG_SDL_CLIPBOARD', have_sdl_clipboard)
> > config_host_data.set('CONFIG_SECCOMP', seccomp.found())
> > if seccomp.found()
> > config_host_data.set('CONFIG_SECCOMP_SYSRAWRC', seccomp_has_sysrawrc)
> > diff -ruN qemu-10.0.0-original/meson_options.txt 
> > qemu-10.0.0-modified/meson_options.txt
> > --- qemu-10.0.0-original/meson_options.txt 2025-07-30 11:52:13
> > +++ qemu-10.0.0-modified/meson_options.txt 2025-07-30 11:58:15
> > @@ -212,6 +212,8 @@
> > description: 'SDL user interface')
> > option('sdl_image', type : 'feature', value : 'auto',
> > description: 'SDL Image support for icons')
> > +option('sdl_clipboard', type : 'boolean', value : true,
> > + description: 'SDL clipboard support')
> > option('seccomp', type : 'feature', value : 'auto',
> > description: 'seccomp support')
> > option('smartcard', type : 'feature', value : 'auto',
> > diff -ruN qemu-10.0.0-original/ui/meson.build 
> > qemu-10.0.0-modified/ui/meson.build
> > --- qemu-10.0.0-original/ui/meson.build 2025-07-30 11:51:58
> > +++ qemu-10.0.0-modified/ui/meson.build 2025-07-30 11:59:00
> > @@ -126,6 +126,9 @@
> > 'sdl2-input.c',
> > 'sdl2.c',
> > ))
> > + if have_sdl_clipboard
> > + sdl_ss.add(files('sdl2-clipboard.c'))
> > + endif
> > sdl_ss.add(when: opengl, if_true: files('sdl2-gl.c'))
> > sdl_ss.add(when: x11, if_true: files('x_keymap.c'))
> > ui_modules += {'sdl' : sdl_ss}
> > diff -ruN qemu-10.0.0-original/ui/sdl2-clipboard.c 
> > qemu-10.0.0-modified/ui/sdl2-clipboard.c
> > --- qemu-10.0.0-original/ui/sdl2-clipboard.c 1970-01-01 02:00:00
> > +++ qemu-10.0.0-modified/ui/sdl2-clipboard.c 2025-07-30 12:13:25
> > @@ -0,0 +1,154 @@
> > +/*
> > + * SDL UI -- clipboard support (improved async version)
> > + *
> > + * Copyright (C) 2023 Kamay Xutax <ad...@xutaxkamay.com>
> > + * Copyright (C) 2025 startergo <starte...@protonmail.com>
> > + *
> > + * SPDX-License-Identifier: GPL-2.0-or-later
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +#include "ui/console.h"
> > +#include "ui/clipboard.h"
> > +#include "ui/sdl2.h"
> > +#include "qemu/log.h"
> > +
> > +#ifdef CONFIG_SDL_CLIPBOARD
> > +
> > +/* Track pending clipboard requests to handle async data */
> > +typedef struct {
> > + struct sdl2_console *scon;
> > + QemuClipboardInfo *info;
> > + QemuClipboardType type;
> > +} SDLClipboardRequest;
> > +
> > +static SDLClipboardRequest *pending_request = NULL;
> > +
> > +static void sdl2_clipboard_clear_pending(void)
> > +{
> > + if (pending_request) {
> > + if (pending_request->info) {
> > + qemu_clipboard_info_unref(pending_request->info);
> > + }
> > + g_free(pending_request);
> > + pending_request = NULL;
> > + }
> > +}
> > +
> > +static void sdl2_clipboard_notify(Notifier *notifier, void *data)
> > +{
> > + QemuClipboardNotify *notify = data;
> > + struct sdl2_console *scon =
> > + container_of(notifier, struct sdl2_console, cbpeer.notifier);
> > + bool self_update = notify->info->owner == &scon->cbpeer;
> > + const char *text_data;
> > + size_t text_size;
> > +
> > + switch (notify->type) {
> > + case QEMU_CLIPBOARD_UPDATE_INFO:
> > + {
> > + /* Skip self-updates to avoid clipboard manager conflicts */
> > + if (self_update) {
> > + return;
> > + }
> > +
> > + if (!notify->info->types[QEMU_CLIPBOARD_TYPE_TEXT].available) {
> > + return;
> > + }
> > +
> > + /* Check if this is completion of our pending request */
> > + if (pending_request && pending_request->info == notify->info &&
> > + pending_request->type == QEMU_CLIPBOARD_TYPE_TEXT) {
> > + sdl2_clipboard_clear_pending();
> > + }
> > +
> > + /* Check if data is available, request asynchronously if not */
> > + if (!notify->info->types[QEMU_CLIPBOARD_TYPE_TEXT].data) {
> > + if (!pending_request) {
> > + pending_request = g_new0(SDLClipboardRequest, 1);
> > + pending_request->scon = scon;
> > + pending_request->info = qemu_clipboard_info_ref(notify->info);
> > + pending_request->type = QEMU_CLIPBOARD_TYPE_TEXT;
> > + qemu_clipboard_request(notify->info, QEMU_CLIPBOARD_TYPE_TEXT);
> > + }
> > + return;
> > + }
> > +
> > + /* Process available data */
> > + text_size = notify->info->types[QEMU_CLIPBOARD_TYPE_TEXT].size;
> > + if (text_size == 0) {
> > + return;
> > + }
> > +
> > + text_data = (const char 
> > *)notify->info->types[QEMU_CLIPBOARD_TYPE_TEXT].data;
> > +
> > + /* Ensure null termination for SDL clipboard */
> > + g_autofree char *text = g_strndup(text_data, text_size);
> > + if (text && text[0] != '\0') {
> > + SDL_SetClipboardText(text);
> > + } else if (!text) {
> > + qemu_log_mask(LOG_GUEST_ERROR,
> > + "SDL clipboard: Failed to allocate memory for clipboard text\n");
> > + }
> > + break;
> > + }
> > + case QEMU_CLIPBOARD_RESET_SERIAL:
> > + sdl2_clipboard_clear_pending();
> > + break;
> > + }
> > +}
> > +
> > +static void sdl2_clipboard_request(QemuClipboardInfo *info,
> > + QemuClipboardType type)
> > +{
> > + g_autofree char *text = NULL;
> > +
> > + if (type != QEMU_CLIPBOARD_TYPE_TEXT) {
> > + return;
> > + }
> > +
> > + text = SDL_GetClipboardText();
> > + if (!text) {
> > + qemu_log_mask(LOG_GUEST_ERROR,
> > + "SDL clipboard: Failed to get clipboard text: %s\n",
> > + SDL_GetError());
> > + return;
> > + }
> > +
> > + qemu_clipboard_set_data(info->owner, info, type,
> > + strlen(text), text, true);
> > +}
> > +
> > +void sdl2_clipboard_init(struct sdl2_console *scon)
> > +{
> > + scon->cbpeer.name = "sdl2-clipboard";
> > + scon->cbpeer.notifier.notify = sdl2_clipboard_notify;
> > + scon->cbpeer.request = sdl2_clipboard_request;
> > +
> > + qemu_clipboard_peer_register(&scon->cbpeer);
> > +}
> > +
> > +void sdl2_clipboard_handle_request(struct sdl2_console *scon)
> > +{
> > + g_autofree char *text = NULL;
> > + QemuClipboardInfo *info;
> > +
> > + text = SDL_GetClipboardText();
> > + if (!text) {
> > + qemu_log_mask(LOG_GUEST_ERROR,
> > + "SDL clipboard: Failed to get clipboard text: %s\n",
> > + SDL_GetError());
> > + return;
> > + }
> > +
> > + if (text[0] == '\0') {
> > + return; /* Ignore empty clipboard */
> > + }
> > +
> > + info = qemu_clipboard_info_new(&scon->cbpeer, 
> > QEMU_CLIPBOARD_SELECTION_CLIPBOARD);
> > + qemu_clipboard_set_data(&scon->cbpeer, info, QEMU_CLIPBOARD_TYPE_TEXT,
> > + strlen(text), text, true);
> > + qemu_clipboard_info_unref(info);
> > +}
> > +
> > +#endif /* CONFIG_SDL_CLIPBOARD */
> > diff -ruN qemu-10.0.0-original/ui/sdl2.c qemu-10.0.0-modified/ui/sdl2.c
> > --- qemu-10.0.0-original/ui/sdl2.c 2025-07-30 11:51:58
> > +++ qemu-10.0.0-modified/ui/sdl2.c 2025-07-30 11:59:22
> > @@ -691,6 +691,11 @@
> > case SDL_WINDOWEVENT:
> > handle_windowevent(ev);
> > break;
> > +#ifdef CONFIG_SDL_CLIPBOARD
> > + case SDL_CLIPBOARDUPDATE:
> > + sdl2_clipboard_handle_request(scon);
> > + break;
> > +#endif
> > default:
> > break;
> > }
> > @@ -900,6 +905,10 @@
> > qemu_console_set_display_gl_ctx(con, &sdl2_console[i].dgc);
> > }
> > register_displaychangelistener(&sdl2_console[i].dcl);
> > +
> > +#ifdef CONFIG_SDL_CLIPBOARD
> > + sdl2_clipboard_init(&sdl2_console[i]);
> > +#endif
> >
> > #if defined(SDL_VIDEO_DRIVER_WINDOWS) || defined(SDL_VIDEO_DRIVER_X11)
> > if (SDL_GetWindowWMInfo(sdl2_console[i].real_window, &info)) {
> >
> >
> >
> >
> >
> > Sent with Proton Mail secure email.
> >
> > On Tuesday, July 29th, 2025 at 3:11 PM, Marc-André Lureau 
> > <marcandre.lur...@gmail.com> wrote:
> >
> > > Hi
> > >
> > > On Mon, Jul 28, 2025 at 5:06 PM startergo starte...@protonmail.com wrote:
> > >
> > > > Subject: Re: [PATCH RFC v1 1/1] ui/sdl2: clipboard sharing 
> > > > implementation for SDL
> > > > In-Reply-To: 
> > > > cakmqykm+4kbqynhkqwxqphyp1zn5crtxc4r7yj8wbx5m+tc...@mail.gmail.com
> > > > References: 20231108105411.1759509-1-ad...@xutaxkamay.com 
> > > > cakmqykm+4kbqynhkqwxqphyp1zn5crtxc4r7yj8wbx5m+tc...@mail.gmail.com
> > > >
> > > > Hi Marc-André,
> > > >
> > > > Following up on your thoughtful review of the SDL clipboard RFC from 
> > > > November 2023,
> > > > I've developed a comprehensive implementation that directly addresses 
> > > > the concerns
> > > > you raised about main loop reentrancy and clipboard management issues.
> > > >
> > > > ## Key Improvements Addressing Your Feedback:
> > > >
> > > > 1. Main Loop Reentrancy Solution
> > > > You correctly identified the problematic `main_loop_wait(false)` 
> > > > pattern from the
> > > > original RFC. My implementation eliminates this entirely by:
> > > > - Using immediate data processing without busy-wait loops
> > > > - Implementing proper asynchronous clipboard handling
> > > > - Following the same safety patterns used in QEMU issue #1150 resolution
> > > >
> > > > 2. Clipboard Manager Conflict Prevention
> > > > Your concern about fighting with clipboard managers is addressed 
> > > > through:
> > > > - Self-update loop prevention in `sdl2_clipboard_update()`
> > > > - Clean ownership tracking via `info->owner == &scon->cbpeer` checks
> > > > - No automatic clipboard stealing or aggressive management behavior
> > > >
> > > > 3. Null Termination Handling
> > > > Regarding your question about proper string handling: my implementation 
> > > > ensures:
> > > > - SDL-compatible null-terminated UTF-8 strings using `g_strndup()`
> > > > - Proper length calculation excluding null terminator for QEMU clipboard
> > > > - Safe handling of embedded nulls in clipboard data
> > > >
> > > > 4. Configuration Options
> > > > Following your suggestion about the optional nature (like 
> > > > gtk_clipboard), the
> > > > implementation includes:
> > > > - `CONFIG_SDL_CLIPBOARD` build option for conditional compilation
> > > > - Clean fallback when clipboard support is disabled
> > > > - No forced dependencies or runtime requirements
> > > >
> > > > ## Technical Implementation Details:
> > > >
> > > > The implementation uses event-driven clipboard monitoring via 
> > > > `SDL_CLIPBOARDUPDATE`
> > > > rather than polling, and integrates cleanly with QEMU's unified 
> > > > clipboard subsystem
> > > > through the `QemuClipboardPeer` interface.
> > > >
> > > > Key safety features:
> > > > - No main loop reentrancy
> > > > - Proper memory management with SDL-specific allocation/deallocation
> > > > - Self-update prevention to avoid clipboard ownership conflicts
> > > > - UTF-8 string validation and proper null termination
> > > >
> > > > ## Testing and Validation:
> > > >
> > > > Extensive testing on macOS with Linux guest demonstrates:
> > > > - Reliable bidirectional clipboard operation
> > > > - No performance impact or stability regressions
> > > > - Clean coexistence with system clipboard managers
> > > > - Proper handling of various text encodings and formats
> > > >
> > > > This implementation addresses the SDL2 backend's clipboard gap while 
> > > > incorporating
> > > > lessons learned from both the GTK clipboard implementation and the 
> > > > community
> > > > feedback from the original RFC.
> > > >
> > > > The patch brings SDL2 to feature parity with other QEMU display 
> > > > backends regarding
> > > > clipboard functionality, using a safety-first approach that avoids the 
> > > > pitfalls
> > > > identified in your review.
> > > >
> > > > Would appreciate your thoughts on this refined approach. The 
> > > > implementation is
> > > > ready for community review and addresses the architectural concerns 
> > > > raised in
> > > > the original thread.
> > > >
> > > > Best regards,
> > > > startergo
> > > >
> > > > ---
> > > >
> > > > [Complete patch follows below]
> > >
> > >
> > > Please send a properly formatted git patch:
> > > https://www.qemu.org/docs/master/devel/submitting-a-patch.html#do-not-send-as-an-attachment
> > >
> > > > From: startergo starte...@protonmail.com
> > > > Date: Mon, 28 Jul 2025 12:00:00 +0000
> > > > Subject: [PATCH] ui/sdl2: Add bidirectional clipboard support
> > > >
> > > > This patch implements bidirectional clipboard functionality for the SDL2
> > > > display backend, addressing the lack of clipboard integration when using
> > > > SDL2 as the display interface.
> > > >
> > > > The implementation addresses concerns raised in previous SDL clipboard
> > > > discussions, particularly around main loop reentrancy and clipboard
> > > > manager conflicts identified in the November 2023 RFC review.Key 
> > > > features:
> > > > - Bidirectional text clipboard synchronization between host and guest
> > > > - Safe implementation avoiding main loop reentrancy issues
> > > > - Proper memory management with SDL-specific allocation/deallocation
> > > > - Integration with QEMU's unified clipboard subsystem
> > > > - Configurable via CONFIG_SDL_CLIPBOARD build option
> > >
> > >
> > > The patch is missing meson updates for a new "sdl_clipboard" option.
> > >
> > > It would not be required if you can avoid the main loop reentrancy.
> > > You removed it, but I am afraid you didn't address the issue from
> > > Kamay's original patch.
> > >
> > > > The implementation follows established QEMU patterns and addresses
> > > > reentrancy concerns similar to those resolved in QEMU issue #1150.
> > > >
> > > > Implementation details:
> > > > - Uses SDL_CLIPBOARDUPDATE events to detect host clipboard changes
> > > > - Implements QemuClipboardPeer interface for guest-to-host direction
> > > > - Avoids busy-wait loops by processing clipboard data immediately
> > > > - Proper UTF-8 handling following SDL2 string conventions
> > > > - Memory management uses SDL_free() for SDL-allocated strings
> > > > - Self-update prevention to avoid clipboard manager conflicts
> > > >
> > > > The patch has been tested extensively on macOS with various guest
> > > > operating systems including Linux and Windows. Clipboard functionality
> > > > works reliably in both directions without performance impact or
> > > > stability issues.
> > > >
> > > > This addresses a significant usability gap in the SDL2 backend, bringing
> > > > it to feature parity with other QEMU display backends regarding 
> > > > clipboard
> > > > functionality.
> > > >
> > > > Signed-off-by: startergo starte...@protonmail.com
> > > > ---
> > > > ui/meson.build | 1 +
> > > > include/ui/sdl2.h | 11 +++
> > > > ui/clipboard.c | 104 ++++++++++++++++++++++++++++++++++++++++++
> > >
> > >
> > > name it sdl2-clipboard.c
> > >
> > > > ui/sdl2.c | 8 ++++
> > > > 4 files changed, 124 insertions(+)
> > > > create mode 100644 ui/clipboard.c
> > > >
> > > > diff --git a/ui/meson.build b/ui/meson.build
> > > > index 92e7e61219..c5e7880ca5 100644
> > > > --- a/ui/meson.build
> > > > +++ b/ui/meson.build
> > > > @@ -59,6 +59,7 @@ if sdl.found()
> > > > softmmu_ss.add(when: 'CONFIG_SDL', if_true: files(
> > > > 'sdl2-2d.c',
> > > > 'sdl2-gl.c',
> > > > + 'clipboard.c',
> > >
> > >
> > > make it conditional on have_sdl_clipboard option
> > >
> > > > 'sdl2-input.c',
> > > > 'sdl2.c'
> > > > ))
> > > > diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
> > > > index 1624ad6938..28a17e7b53 100644
> > > > --- a/include/ui/sdl2.h
> > > > +++ b/include/ui/sdl2.h
> > > > @@ -7,6 +7,10 @@
> > > > # include <SDL.h>
> > > > # include <SDL_syswm.h>
> > > >
> > > > +#ifdef CONFIG_SDL_CLIPBOARD
> > > > +#include "ui/clipboard.h"
> > > > +#endif
> > > > +
> > > > struct sdl2_console {
> > > > DisplayChangeListener dcl;
> > > > DisplaySurface *surface;
> > > > @@ -22,6 +26,10 @@ struct sdl2_console {
> > > > int idle_counter;
> > > > int ignore_hotkeys;
> > > > SDL_GLContext winctx;
> > > > +
> > > > +#ifdef CONFIG_SDL_CLIPBOARD
> > > > + QemuClipboardPeer cbpeer;
> > > > +#endif
> > > > };
> > > >
> > > > void sdl2_window_create(struct sdl2_console *scon);
> > > > @@ -39,4 +47,7 @@ void sdl2_reset_keys(struct sdl2_console *scon);
> > > > void sdl2_process_key(struct sdl2_console *scon,
> > > > SDL_KeyboardEvent *ev);
> > > >
> > > > +void sdl2_clipboard_init(struct sdl2_console *scon);
> > > > +void sdl2_clipboard_handle_request(struct sdl2_console scon);
> > > > +
> > > > #endif / SDL2_H /
> > > > diff --git a/ui/clipboard.c b/ui/clipboard.c
> > > > new file mode 100644
> > > > index 0000000000..98fa9f1c2a
> > > > --- /dev/null
> > > > +++ b/ui/clipboard.c
> > > > @@ -0,0 +1,104 @@
> > > > +/
> > > > + * QEMU SDL2 clipboard implementation
> > > > + *
> > > > + * Copyright (c) 2025 startergo
> > > > + *
> > > > + * Permission is hereby granted, free of charge, to any person 
> > > > obtaining a copy
> > > > + * of this software and associated documentation files (the 
> > > > "Software"), to deal
> > > > + * in the Software without restriction, including without limitation 
> > > > the rights
> > > > + * to use, copy, modify, merge, publish, distribute, sublicense, 
> > > > and/or sell
> > > > + * copies of the Software, and to permit persons to whom the Software 
> > > > is
> > > > + * furnished to do so, subject to the following conditions:
> > > > + *
> > > > + * The above copyright notice and this permission notice shall be 
> > > > included in
> > > > + * all copies or substantial portions of the Software.
> > > > + *
> > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> > > > EXPRESS OR
> > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> > > > MERCHANTABILITY,
> > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT 
> > > > SHALL
> > > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
> > > > OR OTHER
> > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
> > > > ARISING FROM,
> > > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> > > > DEALINGS IN
> > > > + * THE SOFTWARE.
> > > > + */
> > >
> > >
> > > QEMU code has SPDX identifiers and is GPL2+:
> > >
> > > /*
> > > * Copyright ...
> > > *
> > > * SPDX-License-Identifier: GPL-2.0-or-later
> > > */
> > >
> > > > +
> > > > +#include "qemu/osdep.h"
> > > > +#include "ui/sdl2.h"
> > > > +
> > > > +#ifdef CONFIG_SDL_CLIPBOARD
> > > > +
> > > > +static void sdl2_clipboard_update(struct sdl2_console *scon,
> > > > + QemuClipboardInfo info)
> > > > +{
> > > > + QemuClipboardType type = QEMU_CLIPBOARD_TYPE_TEXT;
> > > > + g_autoptr(QemuClipboardData) data = NULL;
> > > > +
> > > > + / Prevent self-update loops /
> > > > + if (info->owner == &scon->cbpeer) {
> > > > + return;
> > > > + }
> > > > +
> > > > + / Only handle text clipboard for now /
> > > > + if (!qemu_clipboard_info_has_type(info, type)) {
> > > > + return;
> > > > + }
> > > > +
> > > > + / Get clipboard data from QEMU */
> > > > + data = qemu_clipboard_request(info, type);
> > > > + if (!data || !data->data || data->size == 0) {
> > > > + return;
> > > > + }
> > >
> > >
> > > Here, Kamay's patch was waiting for the clipboard to be filled. You
> > > can't assume the data is readily available after calling
> > > qemu_clipboard_request(). vdagent code will send a request, and data
> > > can come later asynchronously via VD_AGENT_CLIPBOARD message. The code
> > > must deal with QEMU_CLIPBOARD_UPDATE_INFO notifiers / callbacks and
> > > handle request state tracking to properly deal with this.
> > >
> > > > +
> > > > + /*
> > > > + * Ensure null termination for SDL clipboard.
> > > > + * QEMU clipboard data may not be null-terminated.
> > > > + */
> > > > + g_autofree char *text = g_strndup((const char *)data->data, 
> > > > data->size);
> > >
> > >
> > > casting required here?
> > >
> > > > + if (text) {
> > >
> > >
> > > text can't be NULL if data->data is non-NULL. If we want to handle the
> > >
> > > case anyway, we could have a trace or a warning
> > >
> > > > + SDL_SetClipboardText(text);
> > > > + }
> > > > +}
> > > > +
> > > > +static void sdl2_clipboard_notify(Notifier *notifier,
> > > > + void *data)
> > > > +{
> > > > + QemuClipboardNotify *notify = data;
> > > > + struct sdl2_console scon =
> > > > + container_of(notifier, struct sdl2_console, cbpeer.notifier);
> > > > +
> > > > + switch (notify->type) {
> > > > + case QEMU_CLIPBOARD_UPDATE_INFO:
> > > > + sdl2_clipboard_update(scon, notify->info);
> > > > + break;
> > > > + case QEMU_CLIPBOARD_RESET_SERIAL:
> > > > + / Nothing to do for reset */
> > > > + break;
> > > > + }
> > > > +}
> > > > +
> > > > +static void sdl2_clipboard_request(QemuClipboardInfo *info,
> > > > + QemuClipboardType type)
> > > > +{
> > > > + struct sdl2_console *scon =
> > > > + container_of(info->owner, struct sdl2_console, cbpeer);
> > > > + char *sdl_text = NULL;
> > > > +
> > > > + switch (type) {
> > > > + case QEMU_CLIPBOARD_TYPE_TEXT:
> > > > + if (!SDL_HasClipboardText()) {
> > > > + return;
> > > > + }
> > > > +
> > > > + sdl_text = SDL_GetClipboardText();
> > > > + if (sdl_text && strlen(sdl_text) > 0) {
> > >
> > >
> > > Interesting that SDL decided that empty string is for error reporting.
> > >
> > > Could you simplify the check with sdl_text[0] != '\0' instead? Also
> > > add a warning with SDL_GetError() if it's empty.
> > >
> > > > + /*
> > > > + * SDL guarantees null-terminated UTF-8 strings.
> > > > + * Pass length without null terminator as QEMU clipboard
> > > > + * will handle null termination consistently.
> > > > + /
> > > > + qemu_clipboard_set_data(&scon->cbpeer, info, type,
> > > > + strlen(sdl_text), sdl_text, true);
> > > > + }
> > > > +
> > > > + / Always free SDL-allocated memory */
> > > > + if (sdl_text) {
> > >
> > >
> > > drop the condition, GetClipboardText() should not return NULL, and
> > > SDL_free(NULL) is fine anyway.
> > >
> > > > + SDL_free(sdl_text);
> > > > + }
> > > > + break;
> > > > + default:
> > > > + break;
> > > > + }
> > > > +}
> > > > +
> > > > +void sdl2_clipboard_handle_request(struct sdl2_console *scon)
> > > > +{
> > > > + g_autoptr(QemuClipboardInfo) info =
> > > > + qemu_clipboard_info_new(&scon->cbpeer,
> > > > + QEMU_CLIPBOARD_SELECTION_CLIPBOARD);
> > > > +
> > > > + if (info) {
> > >
> > >
> > > qemu_clipboard_info_new never returns NULL
> > >
> > > > + sdl2_clipboard_request(info, QEMU_CLIPBOARD_TYPE_TEXT);
> > > > + }
> > > > +}
> > > > +
> > > > +void sdl2_clipboard_init(struct sdl2_console scon)
> > > > +{
> > > > + scon->cbpeer.name = "sdl2";
> > > > + scon->cbpeer.notifier.notify = sdl2_clipboard_notify;
> > > > + scon->cbpeer.request = sdl2_clipboard_request;
> > > > +
> > > > + / Register the clipboard peer with QEMU /
> > > > + qemu_clipboard_peer_register(&scon->cbpeer);
> > > > +}
> > > > +
> > > > +#endif / CONFIG_SDL_CLIPBOARD */
> > > > diff --git a/ui/sdl2.c b/ui/sdl2.c
> > > > index 1a83c3b1bf..5678930d3c 100644
> > > > --- a/ui/sdl2.c
> > > > +++ b/ui/sdl2.c
> > > > @@ -691,6 +691,11 @@ void sdl2_poll_events(struct sdl2_console *scon)
> > > > case SDL_WINDOWEVENT:
> > > > handle_windowevent(ev);
> > > > break;
> > > > +#ifdef CONFIG_SDL_CLIPBOARD
> > > > + case SDL_CLIPBOARDUPDATE:
> > > > + sdl2_clipboard_handle_request(scon);
> > > > + break;
> > > > +#endif
> > > > default:
> > > > break;
> > > > }
> > > > @@ -914,6 +919,9 @@ static void sdl2_display_init(DisplayState *ds, 
> > > > DisplayOptions *o)
> > > > qemu_console_set_window_id(con, info.info.x11.window);
> > > > #endif
> > > > }
> > > > +#ifdef CONFIG_SDL_CLIPBOARD
> > > > + sdl2_clipboard_init(&sdl2_console[i]);
> > > > +#endif
> > > > }
> > > >
> > > > #ifdef CONFIG_SDL_IMAGE
> > >
> > >
> > >
> > > --
> > > Marc-André Lureau
>
>
>
> --
> Marc-André Lureau



-- 
Marc-André Lureau

Reply via email to