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

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 26/02/15 16:28, Tim Deegan wrote:
> At 16:11 +0000 on 26 Feb (1424963496), Tim Deegan wrote:
>> Add a check, like the existing check for non-ANSI C in the public
>> headers, that runs the public headers through a C++ compiler to
>> flag non-C++-friendly constructs.
> Oops, this still has the EFI changes in it.  v3, rebased, is on its way.
>
>> Unlike the ANSI C check, we accept GCC-isms (gnu++98), and we also
>> check various tools-only headers.
>>
>> 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.

It is basically no effort, wont (directly) break consumers, and will
make the headers fully friendly (other than extern C, which can be dealt
with using the C++ #include <c$FOO> pattern).

+1 throw this in and be done with the incompatibilities for good.

~Andrew

>
> 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..bb13494 100644
> --- a/xen/include/public/io/ring.h
> +++ b/xen/include/public/io/ring.h
> @@ -111,7 +111,7 @@ struct __name##_sring {                                                 \
>              uint8_t msg;                                                \
>          } tapif_user;                                                   \
>          uint8_t pvt_pad[4];                                             \
> -    } private;                                                          \
> +    } local;                                                            \
>      uint8_t __pad[44];                                                  \
>      union __name##_sring_entry ring[1]; /* variable-length */           \
>  };                                                                      \
> @@ -156,7 +156,7 @@ 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)->local.pvt_pad, 0, sizeof((_s)->local.pvt_pad));  \
>      (void)memset((_s)->__pad, 0, sizeof((_s)->__pad));                  \
>  } while(0)
>  
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel