diff mbox series

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

Message ID 20240605160112.925957-4-gbayer@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series vfio/pci: Support 8-byte PCI loads and stores | expand

Commit Message

Gerd Bayer June 5, 2024, 4:01 p.m. UTC 
Correct spelling of DECLA[RA]TION

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(-)

Comments

Alex Williamson June 18, 2024, 5:20 p.m. UTC | #1 
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

> 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)
>  
> -VFIO_IOWRITE_DECLATION(8)
> -VFIO_IOWRITE_DECLATION(16)
> -VFIO_IOWRITE_DECLATION(32)
> +VFIO_IOWRITE_DECLARATION(8);
> +VFIO_IOWRITE_DECLARATION(16);
> +VFIO_IOWRITE_DECLARATION(32);
>  #ifdef iowrite64
> -VFIO_IOWRITE_DECLATION(64)
> +VFIO_IOWRITE_DECLARATION(64);
>  #endif
>  
> -#define VFIO_IOREAD_DECLATION(size) \
> +#define VFIO_IOREAD_DECLARATION(size) \
>  int vfio_pci_core_ioread##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)
>  
> -VFIO_IOREAD_DECLATION(8)
> -VFIO_IOREAD_DECLATION(16)
> -VFIO_IOREAD_DECLATION(32)
> +VFIO_IOREAD_DECLARATION(8);
> +VFIO_IOREAD_DECLARATION(16);
> +VFIO_IOREAD_DECLARATION(32);
>  #ifdef ioread64
> -VFIO_IOREAD_DECLATION(64)
> +VFIO_IOREAD_DECLARATION(64);
>  #endif
>  
>  #endif /* VFIO_PCI_CORE_H */
Gerd Bayer June 18, 2024, 6:04 p.m. UTC | #2 
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.

Thank you,
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)
> >  
> > -VFIO_IOWRITE_DECLATION(8)
> > -VFIO_IOWRITE_DECLATION(16)
> > -VFIO_IOWRITE_DECLATION(32)
> > +VFIO_IOWRITE_DECLARATION(8);
> > +VFIO_IOWRITE_DECLARATION(16);
> > +VFIO_IOWRITE_DECLARATION(32);
> >  #ifdef iowrite64
> > -VFIO_IOWRITE_DECLATION(64)
> > +VFIO_IOWRITE_DECLARATION(64);
> >  #endif
> >  
> > -#define VFIO_IOREAD_DECLATION(size) \
> > +#define VFIO_IOREAD_DECLARATION(size) \
> >  int vfio_pci_core_ioread##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)
> >  
> > -VFIO_IOREAD_DECLATION(8)
> > -VFIO_IOREAD_DECLATION(16)
> > -VFIO_IOREAD_DECLATION(32)
> > +VFIO_IOREAD_DECLARATION(8);
> > +VFIO_IOREAD_DECLARATION(16);
> > +VFIO_IOREAD_DECLARATION(32);
> >  #ifdef ioread64
> > -VFIO_IOREAD_DECLATION(64)
> > +VFIO_IOREAD_DECLARATION(64);
> >  #endif
> >  
> >  #endif /* VFIO_PCI_CORE_H */
> 
>
Alex Williamson June 18, 2024, 7:01 p.m. UTC | #3 
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,

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.
Thanks,

Alex

> > > 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)
> > >  
> > > -VFIO_IOWRITE_DECLATION(8)
> > > -VFIO_IOWRITE_DECLATION(16)
> > > -VFIO_IOWRITE_DECLATION(32)
> > > +VFIO_IOWRITE_DECLARATION(8);
> > > +VFIO_IOWRITE_DECLARATION(16);
> > > +VFIO_IOWRITE_DECLARATION(32);
> > >  #ifdef iowrite64
> > > -VFIO_IOWRITE_DECLATION(64)
> > > +VFIO_IOWRITE_DECLARATION(64);
> > >  #endif
> > >  
> > > -#define VFIO_IOREAD_DECLATION(size) \
> > > +#define VFIO_IOREAD_DECLARATION(size) \
> > >  int vfio_pci_core_ioread##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)
> > >  
> > > -VFIO_IOREAD_DECLATION(8)
> > > -VFIO_IOREAD_DECLATION(16)
> > > -VFIO_IOREAD_DECLATION(32)
> > > +VFIO_IOREAD_DECLARATION(8);
> > > +VFIO_IOREAD_DECLARATION(16);
> > > +VFIO_IOREAD_DECLARATION(32);
> > >  #ifdef ioread64
> > > -VFIO_IOREAD_DECLATION(64)
> > > +VFIO_IOREAD_DECLARATION(64);
> > >  #endif
> > >  
> > >  #endif /* VFIO_PCI_CORE_H */  
> > 
> >   
>
Gerd Bayer June 19, 2024, 7:58 a.m. UTC | #4 
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)
> > > >
diff mbox series

Patch

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)
 
-VFIO_IOWRITE_DECLATION(8)
-VFIO_IOWRITE_DECLATION(16)
-VFIO_IOWRITE_DECLATION(32)
+VFIO_IOWRITE_DECLARATION(8);
+VFIO_IOWRITE_DECLARATION(16);
+VFIO_IOWRITE_DECLARATION(32);
 #ifdef iowrite64
-VFIO_IOWRITE_DECLATION(64)
+VFIO_IOWRITE_DECLARATION(64);
 #endif
 
-#define VFIO_IOREAD_DECLATION(size) \
+#define VFIO_IOREAD_DECLARATION(size) \
 int vfio_pci_core_ioread##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)
 
-VFIO_IOREAD_DECLATION(8)
-VFIO_IOREAD_DECLATION(16)
-VFIO_IOREAD_DECLATION(32)
+VFIO_IOREAD_DECLARATION(8);
+VFIO_IOREAD_DECLARATION(16);
+VFIO_IOREAD_DECLARATION(32);
 #ifdef ioread64
-VFIO_IOREAD_DECLATION(64)
+VFIO_IOREAD_DECLARATION(64);
 #endif
 
 #endif /* VFIO_PCI_CORE_H */