contrib/vhost-user-input/main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
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
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
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); >
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
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
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 :|
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
© 2016 - 2024 Red Hat, Inc.