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

[Don Slutz <dslutz@verizon.com>]

On 02/26/15 11:24, 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.
> 
> 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.

This sentence and the "-Dprivate=private_is_a_keyword_in_cpp" below
appear to be at odds.  Maybe add something like:

The check ignores the use of 'private'.

> 
> Also tidy up the runes for these checks to be a bit more readable.
> 
> Reported-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> Signed-off-by: Tim Deegan <tim@xen.org>
> Cc: Jan Beulich <JBeulich@suse.com>
> 
> ---
> 

You can add my

Tested-by: Don Slutz <dslutz@verizon.com>

   -Don Slutz

> v3: rebase on staging.
> 
> v2: test more headers;
>     define __XEN_TOOLS__;
>     use g++98 rather than ansi;
>     tidy the makefile for readability;
>     add a missing include to flask_op.h, which uses evtchn_port_t.
> ---
>  .gitignore                        |  1 +
>  config/StdGNU.mk                  |  2 ++
>  config/SunOS.mk                   |  1 +
>  xen/include/Makefile              | 28 ++++++++++++++++++++++++----
>  xen/include/public/xsm/flask_op.h |  2 ++
>  5 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 13ee05b..78958ea 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -233,6 +233,7 @@ xen/arch/*/efi/compat.c
>  xen/arch/*/efi/efi.h
>  xen/arch/*/efi/runtime.c
>  xen/include/headers.chk
> +xen/include/headers++.chk
>  xen/include/asm
>  xen/include/asm-*/asm-offsets.h
>  xen/include/compat/*
> diff --git a/config/StdGNU.mk b/config/StdGNU.mk
> index 4efebe3..e10ed39 100644
> --- a/config/StdGNU.mk
> +++ b/config/StdGNU.mk
> @@ -2,9 +2,11 @@ AS         = $(CROSS_COMPILE)as
>  LD         = $(CROSS_COMPILE)ld
>  ifeq ($(clang),y)
>  CC         = $(CROSS_COMPILE)clang
> +CXX        = $(CROSS_COMPILE)clang++
>  LD_LTO     = $(CROSS_COMPILE)llvm-ld
>  else
>  CC         = $(CROSS_COMPILE)gcc
> +CXX        = $(CROSS_COMPILE)g++
>  LD_LTO     = $(CROSS_COMPILE)ld
>  endif
>  CPP        = $(CC) -E
> diff --git a/config/SunOS.mk b/config/SunOS.mk
> index 3316280..c2be37d 100644
> --- a/config/SunOS.mk
> +++ b/config/SunOS.mk
> @@ -2,6 +2,7 @@ AS         = $(CROSS_COMPILE)gas
>  LD         = $(CROSS_COMPILE)gld
>  CC         = $(CROSS_COMPILE)gcc
>  CPP        = $(CROSS_COMPILE)gcc -E
> +CXX        = $(CROSS_COMPILE)g++
>  AR         = $(CROSS_COMPILE)gar
>  RANLIB     = $(CROSS_COMPILE)granlib
>  NM         = $(CROSS_COMPILE)gnm
> diff --git a/xen/include/Makefile b/xen/include/Makefile
> index 94112d1..d48a642 100644
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -87,13 +87,33 @@ compat/xlat.h: $(addprefix compat/.xlat/,$(xlat-y)) Makefile
>  
>  ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
>  
> -all: headers.chk
> +all: headers.chk headers++.chk
>  
> -headers.chk: $(filter-out public/arch-% public/%ctl.h public/xsm/% public/%hvm/save.h, $(wildcard public/*.h public/*/*.h) $(public-y)) Makefile
> -	for i in $(filter %.h,$^); do $(CC) -ansi -include stdint.h -Wall -W -Werror -S -o /dev/null -x c $$i || exit 1; echo $$i; done >$@.new
> +PUBLIC_HEADERS := $(filter-out public/arch-% public/dom0_ops.h, $(wildcard public/*.h public/*/*.h) $(public-y))
> +
> +PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% public/%hvm/save.h, $(PUBLIC_HEADERS))
> +
> +headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
> +	for i in $(filter %.h,$^); do \
> +	    $(CC) -x c -ansi -Wall -Werror -include stdint.h \
> +	          -S -o /dev/null $$i || exit 1; \
> +	    echo $$i; \
> +	done >$@.new
> +	mv $@.new $@
> +
> +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 \
> +	               -include stdint.h -include public/xen.h \
> +	               -S -o /dev/null $$i || exit 1; \
> +	        echo $$i; \
> +	    done ; \
> +	fi >$@.new
>  	mv $@.new $@
>  
>  endif
>  
>  clean::
> -	rm -rf compat headers.chk
> +	rm -rf compat headers.chk headers++.chk
> diff --git a/xen/include/public/xsm/flask_op.h b/xen/include/public/xsm/flask_op.h
> index 233de81..f874589 100644
> --- a/xen/include/public/xsm/flask_op.h
> +++ b/xen/include/public/xsm/flask_op.h
> @@ -25,6 +25,8 @@
>  #ifndef __FLASK_OP_H__
>  #define __FLASK_OP_H__
>  
> +#include "../event_channel.h"
> +
>  #define XEN_FLASK_INTERFACE_VERSION 1
>  
>  struct xen_flask_load {
>