From mboxrd@z Thu Jan 1 00:00:00 1970 From: Don Slutz Subject: Re: [PATCH v3] RFC: Automatically check xen's public headers for C++ pitfalls. Date: Thu, 26 Feb 2015 15:28:39 -0500 Message-ID: <54EF81F7.4010309@terremark.com> References: <1424956264-15271-1-git-send-email-tim@xen.org> <1424967893-21641-1-git-send-email-tim@xen.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1424967893-21641-1-git-send-email-tim@xen.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tim Deegan , xen-devel@lists.xen.org Cc: Jan Beulich List-Id: xen-devel@lists.xenproject.org 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 > Signed-off-by: Tim Deegan > Cc: Jan Beulich > > --- > You can add my Tested-by: Don Slutz -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 { >