Re: [PATCH v5 2/4] virtio-dmabuf: introduce virtio-dmabuf

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 6/9/23 10:45, Albert Esteve wrote:

>          > diff --git a/include/hw/virtio/virtio-dmabuf.h
>         b/include/hw/virtio/virtio-dmabuf.h
>          > new file mode 100644
>          > index 0000000000..536e622555
>          > --- /dev/null
>          > +++ b/include/hw/virtio/virtio-dmabuf.h
>          > @@ -0,0 +1,103 @@
>          > +/*
>          > + * Virtio Shared dma-buf
>          > + *
>          > + * Copyright Red Hat, Inc. 2023
>          > + *
>          > + * Authors:
>          > + *     Albert Esteve <aesteve@redhat.com
>         <mailto:aesteve@redhat.com>>
>          > + *
>          > + * This work is licensed under the terms of the GNU GPL,
>         version 2.
>          > + * See the COPYING file in the top-level directory.
>          > + */
>          > +
>          > +#ifndef VIRTIO_DMABUF_H
>          > +#define VIRTIO_DMABUF_H
>          > +
>          > +#include "qemu/osdep.h"
>          > +
>          > +#include <glib.h>
>          > +#include "qemu/uuid.h"
>          > +#include "vhost.h"
>          > +
>          > +enum SharedObjectType {
>          > +    TYPE_INVALID = 0,
>          > +    TYPE_DMABUF,
>          > +    TYPE_VHOST_DEV,
>          > +};
>          > +
> 
>         Please declare a
> 
>         typedef
> 
>          > +struct VirtioSharedObject {
>          > +    enum SharedObjectType type;
>          > +    gpointer value;
>          > +};
> 
>         VirtioSharedObject;
> 
>         and use it instead of 'struct VirtioSharedObject'.
> 
> 
>     You mean making the struct anonymous and typedefing?
> 
> 
> So after re-reading your comment and looking for more examples in the 
> codebase, I see
> it is not uncommon to have a named struct also typedef in the same 
> declaration.
> So I will typedef, but not make it anonymous, same for the enum.

Correct (see https://qemu-project.gitlab.io/qemu/devel/style.html#typedefs

>     Should I do the same with the enum? In other files I see enums are
>     typedef too, but not anonymous (e.g., block/qcow2.h).
>     So I could do the same here.
> 
>     For the rest... Ack!

Thanks!

Phil.