diff mbox series

[03/11] UAPI: virtio_net: Fix use of C++ keywords as structural members

Message ID 153616289529.23468.7498785670556620808.stgit@warthog.procyon.org.uk (mailing list archive)
State New, archived
Headers show
Series [01/11] UAPI: drm: Fix use of C++ keywords as structural members | expand

Commit Message

David Howells Sept. 5, 2018, 3:54 p.m. UTC
The virtio_net_ctrl_hdr struct uses a C++ keyword as structural members.  Fix
this by inserting an anonymous union that provides an alternative name and
then hide the reserved name in C++.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: "Michael S. Tsirkin" <mst@redhat.com>
cc: Jason Wang <jasowang@redhat.com>
cc: virtualization@lists.linux-foundation.org
---

 include/uapi/linux/virtio_net.h |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Greg Kroah-Hartman Sept. 5, 2018, 4:54 p.m. UTC | #1
On Wed, Sep 05, 2018 at 04:54:55PM +0100, David Howells wrote:
> The virtio_net_ctrl_hdr struct uses a C++ keyword as structural members.  Fix
> this by inserting an anonymous union that provides an alternative name and
> then hide the reserved name in C++.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: "Michael S. Tsirkin" <mst@redhat.com>
> cc: Jason Wang <jasowang@redhat.com>
> cc: virtualization@lists.linux-foundation.org
> ---
> 
>  include/uapi/linux/virtio_net.h |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index a3715a3224c1..967142bc0e05 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -150,7 +150,12 @@ struct virtio_net_hdr_mrg_rxbuf {
>   * command goes in between.
>   */
>  struct virtio_net_ctrl_hdr {
> -	__u8 class;
> +	union {
> +#ifndef __cplusplus
> +		__u8 class;
> +#endif
> +		__u8 _class;
> +	};

Ugh, ick, no!

Come on now, either put the whole C namespace stuff around the file, or
don't care about this at all.  Doing this whack-a-mole style is a mess.

"class" is a fine variable name for C code, there's no reason this has
to change here at all.

greg k-h
David Howells Sept. 5, 2018, 5:15 p.m. UTC | #2
Greg KH <gregkh@linuxfoundation.org> wrote:

> Come on now, either put the whole C namespace stuff around the file,

You mean wrap it with 'extern "C" { ... }'?  That doesn't fix it.  That only
affects the symbols generated by the compiler.

> "class" is a fine variable name for C code, there's no reason this has
> to change here at all.

I'm trying to prevent future accidents like the one in linux/keyctl.h.  The
easiest way to do this[**] is to pass the entire set of UAPI headers[*]
through the compiler together.

Besides I still have my dark plan to C++-ise the kernel[***] :-D

David

[*] with some obvious exceptions

[**] and it catches other errors too

[***] https://lkml.org/lkml/2018/4/1/116
Michael S. Tsirkin Sept. 5, 2018, 5:35 p.m. UTC | #3
On Wed, Sep 05, 2018 at 04:54:55PM +0100, David Howells wrote:
> The virtio_net_ctrl_hdr struct uses a C++ keyword as structural members.  Fix
> this by inserting an anonymous union that provides an alternative name and
> then hide the reserved name in C++.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: "Michael S. Tsirkin" <mst@redhat.com>
> cc: Jason Wang <jasowang@redhat.com>
> cc: virtualization@lists.linux-foundation.org
> ---
> 
>  include/uapi/linux/virtio_net.h |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index a3715a3224c1..967142bc0e05 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -150,7 +150,12 @@ struct virtio_net_hdr_mrg_rxbuf {
>   * command goes in between.
>   */
>  struct virtio_net_ctrl_hdr {
> -	__u8 class;
> +	union {
> +#ifndef __cplusplus
> +		__u8 class;
> +#endif
> +		__u8 _class;
> +	};
>  	__u8 cmd;
>  } __attribute__((packed));


As long as you do not intend to use any classes, how about
simply adding

-Dclass=_class

to your command line?

Seems to work fine with gcc 8.1.1 on Fedora.
David Howells Sept. 6, 2018, 7:09 a.m. UTC | #4
Michael S. Tsirkin <mst@redhat.com> wrote:

> As long as you do not intend to use any classes, how about
> simply adding
> 
> -Dclass=_class
> 
> to your command line?

That kind of misses the point;-).  It's not reasonable to expect all userspace
C++ users to do this.

David
Michael S. Tsirkin Sept. 6, 2018, 2:36 p.m. UTC | #5
On Thu, Sep 06, 2018 at 08:09:19AM +0100, David Howells wrote:
> Michael S. Tsirkin <mst@redhat.com> wrote:
> 
> > As long as you do not intend to use any classes, how about
> > simply adding
> > 
> > -Dclass=_class
> > 
> > to your command line?
> 
> That kind of misses the point;-).  It's not reasonable to expect all userspace
> C++ users to do this.
> 
> David

I thought one of the points was that building kernel with c++ catches
some bugs, no?  If the point is to make life easier for c++ userspace
I'm not sure what we can do to be frank. C++ seems to be adding new
keywords with no restraint (C99 did it with inline and restrict too, but
it seems this stopped) so no good way to future-proof code for all
language dialects.

So I'd like to know which are the actual c++ users asking for this - we
can then accomodate the specific version they need.
Meanwhile people can get by with a wrapper along the lines of
#define class _class
#include <linux/virtio_net.h>
#undef class
diff mbox series

Patch

diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index a3715a3224c1..967142bc0e05 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -150,7 +150,12 @@  struct virtio_net_hdr_mrg_rxbuf {
  * command goes in between.
  */
 struct virtio_net_ctrl_hdr {
-	__u8 class;
+	union {
+#ifndef __cplusplus
+		__u8 class;
+#endif
+		__u8 _class;
+	};
 	__u8 cmd;
 } __attribute__((packed));