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

["Jan Beulich" <JBeulich@suse.com>]

>>> 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


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