| Message ID | 1490987719-25452-2-git-send-email-sstabellini@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 31.03.17 at 21:15, <sstabellini@kernel.org> 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@aporeto.com> > CC: JBeulich@suse.com > CC: konrad.wilk@oracle.com > --- > .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@suse.com> Jan
On Mon, 3 Apr 2017, Jan Beulich wrote: > >>> On 31.03.17 at 21:15, <sstabellini@kernel.org> 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@aporeto.com> > > CC: JBeulich@suse.com > > CC: konrad.wilk@oracle.com > > --- > > .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@suse.com> Thank you, I can do that! Are you OK also with the other patches, patch #1 in particular?
>>> On 03.04.17 at 20:25, <sstabellini@kernel.org> wrote: > On Mon, 3 Apr 2017, Jan Beulich wrote: >> >>> On 31.03.17 at 21:15, <sstabellini@kernel.org> wrote: >> > @@ -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@suse.com> > > Thank you, I can do that! Are you OK also with the other patches, patch > #1 in particular? I have no further comments on them, but I can't ack them, and I don't feel like giving my R-b for them. Jan
On Mon, 3 Apr 2017, Jan Beulich wrote: > >>> On 31.03.17 at 21:15, <sstabellini@kernel.org> 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@aporeto.com> > > CC: JBeulich@suse.com > > CC: konrad.wilk@oracle.com > > --- > > .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@suse.com> Turns out that I have a small fix to patch #1 to do, so I'll resend the series once again with your comments and reviewed-by. I hope I interpreted your alignment asks correctly.
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;) + 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;) mv $@.new $@ endif @@ -128,5 +134,5 @@ all: $(BASEDIR)/include/asm-x86/cpuid-autogen.h endif clean:: - rm -rf compat headers.chk headers++.chk + rm -rf compat headers*.chk rm -f $(BASEDIR)/include/asm-x86/cpuid-autogen.h
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@aporeto.com> CC: JBeulich@suse.com CC: konrad.wilk@oracle.com --- .gitignore | 3 +-- xen/include/Makefile | 30 ++++++++++++++++++------------ 2 files changed, 19 insertions(+), 14 deletions(-)