Re: [Xen-devel] [PATCH v8 2/4] xen: introduce a C99 headers check

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.03.17 at 21:15, <sstabellini@xxxxxxxxxx> wrote:
> Introduce a C99 headers check, for non-ANSI compliant headers: 9pfs.h
> and pvcalls.h.
> 
> In addition to the usual -include stdint.h, also add -include string.h
> to the C99 check to get the declaration of memcpy and size_t.
> 
> For the same reason, also add -include cstring to the C++ check when
> necessary.
> 
> Signed-off-by: Stefano Stabellini <stefano@xxxxxxxxxxx>
> CC: JBeulich@xxxxxxxx 
> CC: konrad.wilk@xxxxxxxxxx 
> ---
>  .gitignore           |  3 +--
>  xen/include/Makefile | 30 ++++++++++++++++++------------
>  2 files changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 443b12a..0265c1e 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -273,8 +273,7 @@ xen/arch/*/efi/boot.c
>  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/headers*.chk
>  xen/include/asm
>  xen/include/asm-*/asm-offsets.h
>  xen/include/asm-x86/cpuid-autogen.h
> diff --git a/xen/include/Makefile b/xen/include/Makefile
> index aca7f20..fd57ce4 100644
> --- a/xen/include/Makefile
> +++ b/xen/include/Makefile
> @@ -90,11 +90,12 @@ compat/xlat.h: $(addprefix compat/.xlat/,$(xlat-y)) 
> Makefile
>  
>  ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
>  
> -all: headers.chk headers++.chk
> +all: headers.chk headers99.chk headers++.chk
>  
>  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))
> +PUBLIC_C99_HEADERS :=
> +PUBLIC_ANSI_HEADERS := $(filter-out public/%ctl.h public/xsm/% 
> public/%hvm/save.h $(PUBLIC_C99_HEADERS), $(PUBLIC_HEADERS))
>  
>  headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
>       for i in $(filter %.h,$^); do \
> @@ -104,16 +105,21 @@ headers.chk: $(PUBLIC_ANSI_HEADERS) Makefile
>       done >$@.new
>       mv $@.new $@
>  
> +headers99.chk: $(PUBLIC_C99_HEADERS) Makefile
> +     rm -f $@.new
> +     $(foreach i, $(filter %.h,$^), $(CC) -x c -std=c99 -Wall -Werror    \
> +         -include stdint.h $(foreach j, $($(i)-prereq), -include $(j).h) \
> +         -S -o /dev/null $(i) || exit $$?; echo $(i) >> $@.new;)

I would have wished that you formatted this along the lines of
the C++ rule below (|| first on its line, aligned with the beginning
of the command). But anyway - I can live with it here, but ...

> +     mv $@.new $@
> +
>  headers++.chk: $(PUBLIC_HEADERS) Makefile
> -     if $(CXX) -v >/dev/null 2>&1; then \
> -         for i in $(filter %.h,$^); do \
> -             echo '#include "'$$i'"' \
> -             | $(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__ \
> -               -include stdint.h -include public/xen.h -S -o /dev/null - \
> -             || exit 1; \
> -             echo $$i; \
> -         done ; \
> -     fi >$@.new
> +     rm -f $@.new
> +     $(CXX) -v >/dev/null 2>&1 || exit 0;                               \
> +     $(foreach i, $(filter %.h,$^), echo "#include "\"$(i)\"            \
> +         |$(CXX) -x c++ -std=gnu++98 -Wall -Werror -D__XEN_TOOLS__      \
> +         -include stdint.h -include public/xen.h                        \
> +         $(foreach j, $($(i)-prereq), -include c$(j)) -S -o /dev/null - \
> +         || exit $$?; echo $(i) >> $@.new;)

... indentation still doesn't match how it was originally (including,
as mentioned above as well, aligning the start of the command
with | and || ) and there's a blank missing after | . Of course I'm
fine with you fixing this upon commit, if no other need arises for
a v9, so on that basis with those adjustments
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


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