Re: [PATCH v5 3/3] vfio/pci: Fix typo in macro to declare accessors

Gerd Bayer
2024-06-19T07:58:18Z

On Tue, 2024-06-18 at 13:01 -0600, Alex Williamson wrote:
> On Tue, 18 Jun 2024 20:04:26 +0200
> Gerd Bayer <gbayer@linux.ibm.com> wrote:
>
> > On Tue, 2024-06-18 at 11:20 -0600, Alex Williamson wrote:
> > > On Wed,  5 Jun 2024 18:01:12 +0200
> > > Gerd Bayer <gbayer@linux.ibm.com> wrote:
> > >
> > > > Correct spelling of DECLA[RA]TION
> > >
> > > But why did we also transfer the semicolon from the body of the
> > > macro
> > > to the call site?  This doesn't match how we handle macros for
> > > VFIO_IOWRITE, VFIO_IOREAD, or the new VFIO_IORDWR added in this
> > > series.
> > > Thanks,
> > >
> > > Alex
> >
> > Hi Alex,
> >
> > I wanted to make it visible, already in the contracted form, that
> > VFIO_IO{READ|WRITE}_DECLARATION is in fact expanding to a function
> > prototype declaration, while the marco defines in
> > drivers/vfio/pci/vfio_pci_core.c expand to function
> > implementations.
> >
> > My quick searching for in-tree precedence was pretty inconclusive
> > though. So, I can revert that if you want.
>
> Hi Gerd,

Hi Alex,

> I'd tend to keep them as is since both are declaring something, a
> prototype or a function, rather than a macro intended to be used
> inline.  Ideally one macro could handle both declarations now that we
> sort of have symmetry but we'd currently still need a #ifdef in the
> macro which doesn't trivially work.  If we were to do something like
> that though, relocating the semicolon doesn't make sense.
>
> In any case, this proposal is stated as just a typo fix, but it's
> more.

I have no hard feelings about the place of the semicolon - I'll be
sending out a v6 with just the typo fix in patch 3/3.

> Thanks,
>
> Alex

Thanks,
Gerd

>
> > > > Suggested-by: Ramesh Thomas <ramesh.thomas@intel.com>
> > > > Signed-off-by: Gerd Bayer <gbayer@linux.ibm.com>
> > > > ---
> > > >  include/linux/vfio_pci_core.h | 24 ++++++++++++------------
> > > >  1 file changed, 12 insertions(+), 12 deletions(-)
> > > >
> > > > diff --git a/include/linux/vfio_pci_core.h
> > > > b/include/linux/vfio_pci_core.h
> > > > index f4cf5fd2350c..fa59d40573f1 100644
> > > > --- a/include/linux/vfio_pci_core.h
> > > > +++ b/include/linux/vfio_pci_core.h
> > > > @@ -139,26 +139,26 @@ bool
> > > > vfio_pci_core_range_intersect_range(loff_t buf_start, size_t
> > > > buf_cnt,
> > > >   loff_t *buf_offset,
> > > >   size_t
> > > > *intersect_count,
> > > >   size_t
> > > > *register_offset);
> > > > -#define VFIO_IOWRITE_DECLATION(size) \
> > > > +#define VFIO_IOWRITE_DECLARATION(size) \
> > > >  int vfio_pci_core_iowrite##size(struct vfio_pci_core_device
> > > > *vdev, \
> > > > - bool test_mem, u##size val, void
> > > > __iomem
> > > > *io);
> > > > + bool test_mem, u##size val, void
> > > > __iomem
> > > > *io)
> > > >