Re: [Xen-devel] [PATCH v2] RFC: Automatically check xen's public headers for C++ pitfalls.

[Tim Deegan <tim@xxxxxxx>]

At 16:47 +0000 on 26 Feb (1424965651), Jan Beulich wrote:
> >>> On 26.02.15 at 17:28, <tim@xxxxxxx> wrote:
> > At 16:11 +0000 on 26 Feb (1424963496), Tim Deegan wrote:
> >> Explicitly _not_ addressing the use of 'private' in various fields,
> >> since we'd previously decided not to fix that.
> > 
> > BTW, ring.h is the only instance of that, so the extra diff to clear
> > that up too is pretty small (see below).
> > 
> > Not sure what people think about that though - it might be
> > quite a PITA for downstream users of it, though they ought really to
> > be using local copies so they can update in a controlled way.
> 
> linux-2.6.18-xen.hg always having consumed them (almost)
> verbatim, I don't think we should break users not massaging
> the headers. I.e. at least make the field name conditional upon
> using C vs C++.

Something like this?  This is the kind of uglification that I would
like to avoid, though (and I don't like '#define private pvt' much
either).

Tim.

diff --git a/xen/include/Makefile b/xen/include/Makefile
index d48a642..c7a1d52 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -104,8 +104,7 @@ headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
 headers++.chk: $(PUBLIC_HEADERS) Makefile
        if $(CXX) -v >/dev/null 2>&1; then \
            for i in $(filter %.h,$^); do \
-               $(CXX) -x c++ -std=gnu++98 -Wall -Werror \
-                      -D__XEN_TOOLS__ -Dprivate=private_is_a_keyword_in_cpp \
+               $(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__ \
                       -include stdint.h -include public/xen.h \
                       -S -o /dev/null $$i || exit 1; \
                echo $$i; \
diff --git a/xen/include/public/io/ring.h b/xen/include/public/io/ring.h
index 73e13d7..86fb991 100644
--- a/xen/include/public/io/ring.h
+++ b/xen/include/public/io/ring.h
@@ -35,6 +35,15 @@
 #define xen_wmb() wmb()
 #endif
 
+#ifdef __cplusplus
+/* 'private' is a keyword in C++, so we have to use a different name for
+ * private state there.  Leaving the C name alone to avoid unnecessary
+ * pain for the existing users. */
+#define XEN_RING_PRIVATE pvt
+#else
+#define XEN_RING_PRIVATE private
+#endif
+
 typedef unsigned int RING_IDX;
 
 /* Round a 32-bit unsigned constant down to the nearest power of two. */
@@ -111,7 +120,7 @@ struct __name##_sring {                                     
            \
             uint8_t msg;                                                \
         } tapif_user;                                                   \
         uint8_t pvt_pad[4];                                             \
-    } private;                                                          \
+    } XEN_RING_PRIVATE;                                                 \
     uint8_t __pad[44];                                                  \
     union __name##_sring_entry ring[1]; /* variable-length */           \
 };                                                                      \
@@ -156,7 +165,8 @@ typedef struct __name##_back_ring __name##_back_ring_t
 #define SHARED_RING_INIT(_s) do {                                       \
     (_s)->req_prod  = (_s)->rsp_prod  = 0;                              \
     (_s)->req_event = (_s)->rsp_event = 1;                              \
-    (void)memset((_s)->private.pvt_pad, 0, sizeof((_s)->private.pvt_pad)); \
+    (void)memset((_s)->XEN_RING_PRIVATE.pvt_pad, 0,                     \
+                 sizeof((_s)->XEN_RING_PRIVATE.pvt_pad));               \
     (void)memset((_s)->__pad, 0, sizeof((_s)->__pad));                  \
 } while(0)
 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel