From: Gerd Bayer <gbayer@linux.ibm.com>
To: Alex Williamson <alex.williamson@redhat.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>,
Niklas Schnelle <schnelle@linux.ibm.com>,
Ramesh Thomas <ramesh.thomas@intel.com>,
kvm@vger.kernel.org, linux-s390@vger.kernel.org,
Ankit Agrawal <ankita@nvidia.com>,
Yishai Hadas <yishaih@nvidia.com>,
Halil Pasic <pasic@linux.ibm.com>, Ben Segal <bpsegal@us.ibm.com>,
"Tian, Kevin" <kevin.tian@intel.com>,
Julian Ruess <julianr@linux.ibm.com>
Subject: Re: [PATCH v5 3/3] vfio/pci: Fix typo in macro to declare accessors
Date: Wed, 19 Jun 2024 09:58:03 +0200 [thread overview]
Message-ID: <3dbae0f8f2011e2e7bf9589e95b3325217045311.camel@linux.ibm.com> (raw)
In-Reply-To: <20240618130100.4d7a901f.alex.williamson@redhat.com>
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)
> > > >
prev parent reply other threads:[~2024-06-19 7:58 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-05 16:01 [PATCH v5 0/3] vfio/pci: Support 8-byte PCI loads and stores Gerd Bayer
2024-06-05 16:01 ` [PATCH v5 1/3] vfio/pci: Extract duplicated code into macro Gerd Bayer
2024-06-05 16:01 ` [PATCH v5 2/3] vfio/pci: Support 8-byte PCI loads and stores Gerd Bayer
2024-06-05 16:01 ` [PATCH v5 3/3] vfio/pci: Fix typo in macro to declare accessors Gerd Bayer
2024-06-18 17:20 ` Alex Williamson
2024-06-18 18:04 ` Gerd Bayer
2024-06-18 19:01 ` Alex Williamson
2024-06-19 7:58 ` Gerd Bayer [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3dbae0f8f2011e2e7bf9589e95b3325217045311.camel@linux.ibm.com \
--to=gbayer@linux.ibm.com \
--cc=alex.williamson@redhat.com \
--cc=ankita@nvidia.com \
--cc=bpsegal@us.ibm.com \
--cc=jgg@ziepe.ca \
--cc=julianr@linux.ibm.com \
--cc=kevin.tian@intel.com \
--cc=kvm@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=pasic@linux.ibm.com \
--cc=ramesh.thomas@intel.com \
--cc=schnelle@linux.ibm.com \
--cc=yishaih@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox