Document that virtqueue_pop() returned memory must be released
with free().

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/virtio/virtio.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b69d517496..c6e3bfc500 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -199,6 +199,14 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
                     unsigned int len, unsigned int idx);
 
 void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem);
+/**
+ * virtqueue_pop:
+ * @vq: a VirtQueue queue
+ * @sz: the size of struct to return (must be >= VirtQueueElement)
+ *
+ * Returns: a VirtQueueElement filled from the queue or NULL.
+ * The returned element must be free()-d by the caller.
+ */
 void *virtqueue_pop(VirtQueue *vq, size_t sz);
 unsigned int virtqueue_drop_all(VirtQueue *vq);
 void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz);
-- 
2.21.1

On Mon, Mar 23, 2020 at 12:30 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Document that virtqueue_pop() returned memory must be released
> with free().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  include/hw/virtio/virtio.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b69d517496..c6e3bfc500 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -199,6 +199,14 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
>                      unsigned int len, unsigned int idx);
>
>  void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem);
> +/**
> + * virtqueue_pop:
> + * @vq: a VirtQueue queue
> + * @sz: the size of struct to return (must be >= VirtQueueElement)
> + *
> + * Returns: a VirtQueueElement filled from the queue or NULL.
> + * The returned element must be free()-d by the caller.
> + */
>  void *virtqueue_pop(VirtQueue *vq, size_t sz);
>  unsigned int virtqueue_drop_all(VirtQueue *vq);
>  void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz);
> --
> 2.21.1
>

On Mon, Mar 23, 2020 at 12:55 PM Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> On Mon, Mar 23, 2020 at 12:30 PM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
> >
> > Document that virtqueue_pop() returned memory must be released
> > with free().
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

nack, hw/virtio/virtio.c uses g_malloc

>
>
> > ---
> >  include/hw/virtio/virtio.h | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index b69d517496..c6e3bfc500 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -199,6 +199,14 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
> >                      unsigned int len, unsigned int idx);
> >
> >  void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem);
> > +/**
> > + * virtqueue_pop:
> > + * @vq: a VirtQueue queue
> > + * @sz: the size of struct to return (must be >= VirtQueueElement)
> > + *
> > + * Returns: a VirtQueueElement filled from the queue or NULL.
> > + * The returned element must be free()-d by the caller.
> > + */
> >  void *virtqueue_pop(VirtQueue *vq, size_t sz);
> >  unsigned int virtqueue_drop_all(VirtQueue *vq);
> >  void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz);
> > --
> > 2.21.1
> >
>
>


-- 
Marc-André Lureau

On 3/25/20 6:50 PM, Marc-André Lureau wrote:
> On Mon, Mar 23, 2020 at 12:55 PM Marc-André Lureau
> <marcandre.lureau@redhat.com> wrote:
>>
>> On Mon, Mar 23, 2020 at 12:30 PM Philippe Mathieu-Daudé
>> <philmd@redhat.com> wrote:
>>>
>>> Document that virtqueue_pop() returned memory must be released
>>> with free().
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> nack, hw/virtio/virtio.c uses g_malloc

Indeed... I opened hw/virtio/ and contrib/libvhost-user/ in the geany 
editor, and when I navigated from virtio.c to the 
virtqueue_alloc_element() definition I ended in libvhost-user.c:

static void *
virtqueue_alloc_element(size_t sz,
                                      unsigned out_num, unsigned in_num)
{
     VuVirtqElement *elem;
     size_t in_sg_ofs = ALIGN_UP(sz, __alignof__(elem->in_sg[0]));
     size_t out_sg_ofs = in_sg_ofs + in_num * sizeof(elem->in_sg[0]);
     size_t out_sg_end = out_sg_ofs + out_num * sizeof(elem->out_sg[0]);

     assert(sz >= sizeof(VuVirtqElement));
     elem = malloc(out_sg_end);
     elem->out_num = out_num;
     elem->in_num = in_num;
     elem->in_sg = (void *)elem + in_sg_ofs;
     elem->out_sg = (void *)elem + out_sg_ofs;
     return elem;
}

> 
>>
>>
>>> ---
>>>   include/hw/virtio/virtio.h | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>>> index b69d517496..c6e3bfc500 100644
>>> --- a/include/hw/virtio/virtio.h
>>> +++ b/include/hw/virtio/virtio.h
>>> @@ -199,6 +199,14 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
>>>                       unsigned int len, unsigned int idx);
>>>
>>>   void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem);
>>> +/**
>>> + * virtqueue_pop:
>>> + * @vq: a VirtQueue queue
>>> + * @sz: the size of struct to return (must be >= VirtQueueElement)
>>> + *
>>> + * Returns: a VirtQueueElement filled from the queue or NULL.
>>> + * The returned element must be free()-d by the caller.
>>> + */
>>>   void *virtqueue_pop(VirtQueue *vq, size_t sz);
>>>   unsigned int virtqueue_drop_all(VirtQueue *vq);
>>>   void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz);
>>> --
>>> 2.21.1
>>>
>>
>>
> 
>