Re: [PATCH 7/8] hw/9pfs: annotate V9fsTransport callbacks as coroutine_fn

[Philippe Mathieu-Daudé <philmd@xxxxxxxxxxxxxxxx>]

On 20/7/26 16:02, Christian Schoenebeck wrote:
> On Monday, 20 July 2026 09:55:36 CEST Marc-André Lureau wrote:
> > All V9fsTransport callbacks are invoked exclusively from coroutine
> > context (the v9fs_* PDU handlers). Annotate the function pointer
> > types in V9fsTransport and all implementations (virtio and xen
> > backends), as well as intermediate callers in 9p.c (pdu_marshal,
> > pdu_unmarshal, v9fs_init_qiov_from_pdu, etc.).
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxxx>
> > ---
> >   hw/9pfs/9p.h               | 26 +++++++++++++++-----------
> >   hw/9pfs/9p.c               | 31 +++++++++++++++++++------------
> >   hw/9pfs/virtio-9p-device.c | 24 +++++++++++++-----------
> >   hw/9pfs/xen-9p-backend.c   | 34 +++++++++++++++++-----------------
> >   4 files changed, 64 insertions(+), 51 deletions(-)
> >
> > diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> > index 1a309664f6e..0e52ffbdf38 100644
> > --- a/hw/9pfs/9p.h
> > +++ b/hw/9pfs/9p.h
> > @@ -472,17 +472,21 @@ void pdu_submit(V9fsPDU *pdu, P9MsgHeader *hdr);
> >   void v9fs_reset(V9fsState *s);
> >
> >
> >   struct V9fsTransport {
> > -    ssize_t     (*pdu_vmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
> > -                                va_list ap);
> > -    ssize_t     (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset, const char *fmt,
> > -                                  va_list ap);
> > -    void        (*init_in_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > -                                        unsigned int *pniov, size_t size);
> > -    void        (*init_out_iov_from_pdu)(V9fsPDU *pdu, struct iovec **piov,
> > -                                         unsigned int *pniov, size_t size);
> > -    void        (*push_and_notify)(V9fsPDU *pdu);
> > -    size_t      (*msize_limit)(V9fsState *s);
> > -    size_t      (*response_buffer_size)(V9fsPDU *pdu);
> > +    ssize_t     coroutine_fn (*pdu_vmarshal)(V9fsPDU *pdu, size_t offset,
> > +                                             const char *fmt, va_list ap);
> > +    ssize_t     coroutine_fn (*pdu_vunmarshal)(V9fsPDU *pdu, size_t offset,
> > +                                               const char *fmt, va_list ap);
> > +    void        coroutine_fn (*init_in_iov_from_pdu)(V9fsPDU *pdu,
> > +                                                     struct iovec **piov,
> > +                                                     unsigned int *pniov,
> > +                                                     size_t size);
> > +    void        coroutine_fn (*init_out_iov_from_pdu)(V9fsPDU *pdu,
> > +                                                      struct iovec **piov,
> > +                                                      unsigned int *pniov,
> > +                                                      size_t size);
> > +    void        coroutine_fn (*push_and_notify)(V9fsPDU *pdu);
> > +    size_t      coroutine_fn (*msize_limit)(V9fsState *s);
> > +    size_t      coroutine_fn (*response_buffer_size)(V9fsPDU *pdu);
> >   };
> I wonder whether that could be made more diff/blame-friendly. Especially as
> this currently only serves documentation purposes.
>
> They way you are positioning coroutine_fn it usually resolves to an attribute
> attached to the function's return type, not to the function itself, even
> though clang/gcc handling it flexible. For attaching an attribute to the
> function itself the attribute should either be attached before the function
> declaration or after the function declaration.
I tend to aggree it would be clearer to have these attributes /before/
the function prototype declaration.

> In both cases you could then
> add it to its own, separate line, avoiding a lot of noise.
>
> /Christian