* [PATCH] vhost-user-input: use free(elem) instead of g_free(elem)
@ 2019-11-19 11:16 Stefan Hajnoczi
2019-11-19 11:52 ` Marc-André Lureau
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2019-11-19 11:16 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann, Stefan Hajnoczi, Michael S. Tsirkin
The virtqueue element returned by vu_queue_pop() is allocated using
malloc(3) by virtqueue_alloc_element(). Use the matching free(3)
function instead of glib's g_free().
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
contrib/vhost-user-input/main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c
index 449fd2171a..ef4b7769f2 100644
--- a/contrib/vhost-user-input/main.c
+++ b/contrib/vhost-user-input/main.c
@@ -77,7 +77,7 @@ static void vi_input_send(VuInput *vi, struct virtio_input_event *event)
len = iov_from_buf(elem->in_sg, elem->in_num,
0, &vi->queue[i].event, sizeof(virtio_input_event));
vu_queue_push(dev, vq, elem, len);
- g_free(elem);
+ free(elem);
}
vu_queue_notify(&vi->dev.parent, vq);
@@ -153,7 +153,7 @@ static void vi_handle_sts(VuDev *dev, int qidx)
0, &event, sizeof(event));
vi_handle_status(vi, &event);
vu_queue_push(dev, vq, elem, len);
- g_free(elem);
+ free(elem);
}
vu_queue_notify(&vi->dev.parent, vq);
--
2.23.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] vhost-user-input: use free(elem) instead of g_free(elem)
2019-11-19 11:16 [PATCH] vhost-user-input: use free(elem) instead of g_free(elem) Stefan Hajnoczi
@ 2019-11-19 11:52 ` Marc-André Lureau
2019-11-20 9:37 ` Philippe Mathieu-Daudé
2019-11-20 11:48 ` Daniel P. Berrangé
2 siblings, 0 replies; 7+ messages in thread
From: Marc-André Lureau @ 2019-11-19 11:52 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Michael S. Tsirkin, QEMU, Gerd Hoffmann
On Tue, Nov 19, 2019 at 3:16 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
>
> The virtqueue element returned by vu_queue_pop() is allocated using
> malloc(3) by virtqueue_alloc_element(). Use the matching free(3)
> function instead of glib's g_free().
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> contrib/vhost-user-input/main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c
> index 449fd2171a..ef4b7769f2 100644
> --- a/contrib/vhost-user-input/main.c
> +++ b/contrib/vhost-user-input/main.c
> @@ -77,7 +77,7 @@ static void vi_input_send(VuInput *vi, struct virtio_input_event *event)
> len = iov_from_buf(elem->in_sg, elem->in_num,
> 0, &vi->queue[i].event, sizeof(virtio_input_event));
> vu_queue_push(dev, vq, elem, len);
> - g_free(elem);
> + free(elem);
> }
>
> vu_queue_notify(&vi->dev.parent, vq);
> @@ -153,7 +153,7 @@ static void vi_handle_sts(VuDev *dev, int qidx)
> 0, &event, sizeof(event));
> vi_handle_status(vi, &event);
> vu_queue_push(dev, vq, elem, len);
> - g_free(elem);
> + free(elem);
> }
>
> vu_queue_notify(&vi->dev.parent, vq);
> --
> 2.23.0
>
>
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vhost-user-input: use free(elem) instead of g_free(elem)
2019-11-19 11:16 [PATCH] vhost-user-input: use free(elem) instead of g_free(elem) Stefan Hajnoczi
2019-11-19 11:52 ` Marc-André Lureau
@ 2019-11-20 9:37 ` Philippe Mathieu-Daudé
2019-11-20 11:41 ` Stefan Hajnoczi
2019-11-20 11:48 ` Daniel P. Berrangé
2 siblings, 1 reply; 7+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-11-20 9:37 UTC (permalink / raw)
To: Stefan Hajnoczi, qemu-devel; +Cc: Gerd Hoffmann, Michael S. Tsirkin
On 11/19/19 12:16 PM, Stefan Hajnoczi wrote:
> The virtqueue element returned by vu_queue_pop() is allocated using
> malloc(3) by virtqueue_alloc_element(). Use the matching free(3)
> function instead of glib's g_free().
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Fixes: 06914c97d3a ?
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> contrib/vhost-user-input/main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c
> index 449fd2171a..ef4b7769f2 100644
> --- a/contrib/vhost-user-input/main.c
> +++ b/contrib/vhost-user-input/main.c
> @@ -77,7 +77,7 @@ static void vi_input_send(VuInput *vi, struct virtio_input_event *event)
> len = iov_from_buf(elem->in_sg, elem->in_num,
> 0, &vi->queue[i].event, sizeof(virtio_input_event));
> vu_queue_push(dev, vq, elem, len);
> - g_free(elem);
> + free(elem);
> }
>
> vu_queue_notify(&vi->dev.parent, vq);
> @@ -153,7 +153,7 @@ static void vi_handle_sts(VuDev *dev, int qidx)
> 0, &event, sizeof(event));
> vi_handle_status(vi, &event);
> vu_queue_push(dev, vq, elem, len);
> - g_free(elem);
> + free(elem);
> }
>
> vu_queue_notify(&vi->dev.parent, vq);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vhost-user-input: use free(elem) instead of g_free(elem)
2019-11-20 9:37 ` Philippe Mathieu-Daudé
@ 2019-11-20 11:41 ` Stefan Hajnoczi
2019-11-20 12:04 ` Michael S. Tsirkin
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Hajnoczi @ 2019-11-20 11:41 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Michael S. Tsirkin, qemu-devel, Gerd Hoffmann
[-- Attachment #1: Type: text/plain, Size: 452 bytes --]
On Wed, Nov 20, 2019 at 10:37:35AM +0100, Philippe Mathieu-Daudé wrote:
> On 11/19/19 12:16 PM, Stefan Hajnoczi wrote:
> > The virtqueue element returned by vu_queue_pop() is allocated using
> > malloc(3) by virtqueue_alloc_element(). Use the matching free(3)
> > function instead of glib's g_free().
> >
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>
> Fixes: 06914c97d3a ?
Good idea, I should have included that.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vhost-user-input: use free(elem) instead of g_free(elem)
2019-11-19 11:16 [PATCH] vhost-user-input: use free(elem) instead of g_free(elem) Stefan Hajnoczi
2019-11-19 11:52 ` Marc-André Lureau
2019-11-20 9:37 ` Philippe Mathieu-Daudé
@ 2019-11-20 11:48 ` Daniel P. Berrangé
2019-11-21 13:51 ` Stefan Hajnoczi
2 siblings, 1 reply; 7+ messages in thread
From: Daniel P. Berrangé @ 2019-11-20 11:48 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Michael S. Tsirkin, qemu-devel, Gerd Hoffmann
On Tue, Nov 19, 2019 at 11:16:26AM +0000, Stefan Hajnoczi wrote:
> The virtqueue element returned by vu_queue_pop() is allocated using
> malloc(3) by virtqueue_alloc_element(). Use the matching free(3)
> function instead of glib's g_free().
Just as an FYI, since glib 2.46 g_malloc is hardcoded to use the
system allocator, so it is now guaranteed that g_malloc/malloc
and g_free/free are safely interchangable. I recently got this
clarified in the glib docs:
https://gitlab.gnome.org/GNOME/glib/merge_requests/1099//diffs
QEMU mandates 2.48 so we are now safe in that regard
For readability/sanity sake I'd still suggest matching functions
but it is not a functional danger any more. Even when it was a
risk, that risk only arose if you called GLib's API for installing
a custom allocator callback which QEMU never did, so it was always
a non-issue.
Where this is most helpful is in exchanging allocated data with
3rd party libraries that don't use glib. We no longer have to
worry about dup'ing memory going in/out libraries not using
glib's allocators.
>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> contrib/vhost-user-input/main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/vhost-user-input/main.c b/contrib/vhost-user-input/main.c
> index 449fd2171a..ef4b7769f2 100644
> --- a/contrib/vhost-user-input/main.c
> +++ b/contrib/vhost-user-input/main.c
> @@ -77,7 +77,7 @@ static void vi_input_send(VuInput *vi, struct virtio_input_event *event)
> len = iov_from_buf(elem->in_sg, elem->in_num,
> 0, &vi->queue[i].event, sizeof(virtio_input_event));
> vu_queue_push(dev, vq, elem, len);
> - g_free(elem);
> + free(elem);
> }
>
> vu_queue_notify(&vi->dev.parent, vq);
> @@ -153,7 +153,7 @@ static void vi_handle_sts(VuDev *dev, int qidx)
> 0, &event, sizeof(event));
> vi_handle_status(vi, &event);
> vu_queue_push(dev, vq, elem, len);
> - g_free(elem);
> + free(elem);
> }
>
> vu_queue_notify(&vi->dev.parent, vq);
> --
> 2.23.0
>
>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vhost-user-input: use free(elem) instead of g_free(elem)
2019-11-20 11:41 ` Stefan Hajnoczi
@ 2019-11-20 12:04 ` Michael S. Tsirkin
0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2019-11-20 12:04 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: Philippe Mathieu-Daudé, qemu-devel, Gerd Hoffmann
On Wed, Nov 20, 2019 at 11:41:32AM +0000, Stefan Hajnoczi wrote:
> On Wed, Nov 20, 2019 at 10:37:35AM +0100, Philippe Mathieu-Daudé wrote:
> > On 11/19/19 12:16 PM, Stefan Hajnoczi wrote:
> > > The virtqueue element returned by vu_queue_pop() is allocated using
> > > malloc(3) by virtqueue_alloc_element(). Use the matching free(3)
> > > function instead of glib's g_free().
> > >
> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> >
> > Fixes: 06914c97d3a ?
>
> Good idea, I should have included that.
>
> Stefan
I'd prefer keeping the Fixes tag to bugfixes as opposed to cleanups.
I.e. it should be a reasonable heuristic for people asking "do I need
this patch" to be able to answer based on whether they have the
linked patch.
--
MST
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vhost-user-input: use free(elem) instead of g_free(elem)
2019-11-20 11:48 ` Daniel P. Berrangé
@ 2019-11-21 13:51 ` Stefan Hajnoczi
0 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2019-11-21 13:51 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Gerd Hoffmann, qemu-devel, Stefan Hajnoczi, Michael S. Tsirkin
[-- Attachment #1: Type: text/plain, Size: 1315 bytes --]
On Wed, Nov 20, 2019 at 11:48:56AM +0000, Daniel P. Berrangé wrote:
> On Tue, Nov 19, 2019 at 11:16:26AM +0000, Stefan Hajnoczi wrote:
> > The virtqueue element returned by vu_queue_pop() is allocated using
> > malloc(3) by virtqueue_alloc_element(). Use the matching free(3)
> > function instead of glib's g_free().
>
> Just as an FYI, since glib 2.46 g_malloc is hardcoded to use the
> system allocator, so it is now guaranteed that g_malloc/malloc
> and g_free/free are safely interchangable. I recently got this
> clarified in the glib docs:
>
> https://gitlab.gnome.org/GNOME/glib/merge_requests/1099//diffs
>
> QEMU mandates 2.48 so we are now safe in that regard
>
> For readability/sanity sake I'd still suggest matching functions
> but it is not a functional danger any more. Even when it was a
> risk, that risk only arose if you called GLib's API for installing
> a custom allocator callback which QEMU never did, so it was always
> a non-issue.
You are right, although QEMU did use g_mem_set_vtable(). The custom
functions still used malloc() underneath though so it would be safe
anyway:
commit 98cf48f60aa4999f5b2808569a193a401a390e6a
Author: Paolo Bonzini <pbonzini@redhat.com>
Date: Wed Sep 16 17:38:44 2015 +0200
trace: remove malloc tracing
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-11-21 13:52 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-19 11:16 [PATCH] vhost-user-input: use free(elem) instead of g_free(elem) Stefan Hajnoczi
2019-11-19 11:52 ` Marc-André Lureau
2019-11-20 9:37 ` Philippe Mathieu-Daudé
2019-11-20 11:41 ` Stefan Hajnoczi
2019-11-20 12:04 ` Michael S. Tsirkin
2019-11-20 11:48 ` Daniel P. Berrangé
2019-11-21 13:51 ` Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.