Hi
On Sat, Oct 5, 2024 at 12:50 PM Akihiko Odaki <akihiko.od...@daynix.com> wrote:
>
> On 2024/10/03 20:22, marcandre.lur...@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lur...@redhat.com>
> >
> > Learn to free memfd-allocated shared memory.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> > ---
> > include/ui/qemu-pixman.h | 2 +-
> > hw/display/virtio-gpu.c | 4 ++--
> > ui/console.c | 2 +-
> > ui/qemu-pixman.c | 20 ++++++++++++--------
> > 4 files changed, 16 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
> > index e3dd72b9e3..a97f56d09a 100644
> > --- a/include/ui/qemu-pixman.h
> > +++ b/include/ui/qemu-pixman.h
> > @@ -97,7 +97,7 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph,
> >
> > void qemu_pixman_image_unref(pixman_image_t *image);
> >
> > -void qemu_pixman_win32_image_destroy(pixman_image_t *image, void *data);
> > +void qemu_pixman_shared_image_destroy(pixman_image_t *image, void *data);
> >
> > G_DEFINE_AUTOPTR_CLEANUP_FUNC(pixman_image_t, qemu_pixman_image_unref)
> >
> > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> > index 017a0f170c..77f6e76f23 100644
> > --- a/hw/display/virtio-gpu.c
> > +++ b/hw/display/virtio-gpu.c
> > @@ -298,7 +298,7 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
> > bits, c2d.height ? res->hostmem / c2d.height : 0);
> > #ifdef WIN32
> > if (res->image) {
> > - pixman_image_set_destroy_function(res->image,
> > qemu_pixman_win32_image_destroy, res->handle);
> > + pixman_image_set_destroy_function(res->image,
> > qemu_pixman_shared_image_destroy, res->handle);
> > }
> > #endif
> > }
> > @@ -1317,7 +1317,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque,
> > size_t size,
> > return -EINVAL;
> > }
> > #ifdef WIN32
> > - pixman_image_set_destroy_function(res->image,
> > qemu_pixman_win32_image_destroy, res->handle);
> > + pixman_image_set_destroy_function(res->image,
> > qemu_pixman_shared_image_destroy, res->handle);
> > #endif
> >
> > res->addrs = g_new(uint64_t, res->iov_cnt);
> > diff --git a/ui/console.c b/ui/console.c
> > index 8f416ff0b9..fdd76c2be4 100644
> > --- a/ui/console.c
> > +++ b/ui/console.c
> > @@ -487,7 +487,7 @@ DisplaySurface *qemu_create_displaysurface(int width,
> > int height)
> > #ifdef WIN32
> > qemu_displaysurface_win32_set_handle(surface, handle, 0);
> > pixman_image_set_destroy_function(surface->image,
> > - qemu_pixman_win32_image_destroy,
> > handle);
> > + qemu_pixman_shared_image_destroy,
> > handle);
> > #endif
> > return surface;
> > }
> > diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
> > index 3870e1a215..43050ab7c5 100644
> > --- a/ui/qemu-pixman.c
> > +++ b/ui/qemu-pixman.c
> > @@ -6,6 +6,7 @@
> > #include "qemu/osdep.h"
> > #include "qapi/error.h"
> > #include "ui/console.h"
> > +#include "qemu/memfd.h"
> > #include "standard-headers/drm/drm_fourcc.h"
> > #include "trace.h"
> >
> > @@ -269,16 +270,19 @@ void qemu_pixman_glyph_render(pixman_image_t *glyph,
> > }
> > #endif /* CONFIG_PIXMAN */
> >
> > -#ifdef WIN32
> > void
> > -qemu_pixman_win32_image_destroy(pixman_image_t *image, void *data)
> > +qemu_pixman_shared_image_destroy(pixman_image_t *image, void *data)
> > {
> > + void *ptr = pixman_image_get_data(image);
> > +
> > +#ifndef WIN32
>
> I think it is better to have #ifdef instead of #ifndef. It is a common
> pattern to have #ifdef and #elif defined() for the platform-specific
> code and follow them with #else that implements the generic code. This
> allows supporting multiple platform conditions.
indeed! thanks
>
> > + int shmfd = GPOINTER_TO_INT(data);
> > + size_t size = pixman_image_get_height(image) *
> > pixman_image_get_stride(image);
> > +
> > + qemu_memfd_free(ptr, size, shmfd);
> > +#else
> > HANDLE handle = data;
> >
> > - qemu_win32_map_free(
> > - pixman_image_get_data(image),
> > - handle,
> > - &error_warn
> > - );
> > -}
> > + qemu_win32_map_free(ptr, handle, &error_warn);
> > #endif
> > +}
>
