| Message ID | 1491398035-13184-1-git-send-email-ian.jackson@eu.citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 05/04/17 14:13, Ian Jackson wrote: > This suppresses a warning from GCC7 > xenstored_domain.c:949:32: error: increment of a boolean expression > > We have not yet 100% concluded that this should not be fixed by > changing the `_Bool b; b++;' to `b = 1'. But this fixes the warning > and thus the build for now, and we want to apply the XSA-206 patches > to stable trees. > > In the stable trees, this should be applied _before_ the XSA-206 > series. > > Reported-by: Michael Young <m.a.young@durham.ac.uk> > CC: Wei Liu <wei.liu2@citrix.com> > CC: Jan Beulich <JBeulich@suse.com> > Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> I'm fairly sure this will break the Clang build. Furthermore, the proper fix here is to make less "clever" code. IMO, this compiler warning is entirely legitimate, because relying on the saturation behaviour of incrementing a boolean is worthy only for obfuscated code. ~Andrew
Andrew Cooper writes ("Re: [Xen-devel] [PATCH] tools/xenstore: Add -Wno-bool-operation"):
> I'm fairly sure this will break the Clang build.
This seems unlikely. I assume that clang has the same behaviour as
gcc, for -Wno-unknown-warning.
> Furthermore, the proper fix here is to make less "clever" code. IMO,
> this compiler warning is entirely legitimate, because relying on the
> saturation behaviour of incrementing a boolean is worthy only for
> obfuscated code.
I don't care to argue about this. Feel free to submit an alternative
patch.
Ian.
On 05/04/17 14:27, Ian Jackson wrote: > Andrew Cooper writes ("Re: [Xen-devel] [PATCH] tools/xenstore: Add -Wno-bool-operation"): >> I'm fairly sure this will break the Clang build. > This seems unlikely. Have you tried? > I assume that clang has the same behaviour as gcc, for -Wno-unknown-warning. Have you tried? And independently of that, andrewcoop@andrewcoop:/local/xen.git/xen$ git grep unknown-warning -- :/ andrewcoop@andrewcoop:/local/xen.git/xen$ > >> Furthermore, the proper fix here is to make less "clever" code. IMO, >> this compiler warning is entirely legitimate, because relying on the >> saturation behaviour of incrementing a boolean is worthy only for >> obfuscated code. > I don't care to argue about this. Feel free to submit an alternative > patch. You have already identified the correct fix, which both makes the code clearer to read, and compiles properly. What is wrong with that? ~Andrew
On 05/04/17 14:43, Andrew Cooper wrote: > On 05/04/17 14:27, Ian Jackson wrote: >> Andrew Cooper writes ("Re: [Xen-devel] [PATCH] tools/xenstore: Add -Wno-bool-operation"): >>> I'm fairly sure this will break the Clang build. >> This seems unlikely. > Have you tried? In an attempt to not be completely unhelpful in this situation, xenstore_client.o xenstore_client.c error: unknown warning option '-Wno-bool-operation'; did you mean '-Wno-bool-conversion'? [-Werror,-Wunknown-warning-option] /local/xen.git/tools/xenstore/../../tools/Rules.mk:212: recipe for target 'xenstore_client.o' failed > >> I assume that clang has the same behaviour as gcc, for -Wno-unknown-warning. > Have you tried? > > And independently of that, > > andrewcoop@andrewcoop:/local/xen.git/xen$ git grep unknown-warning -- :/ > andrewcoop@andrewcoop:/local/xen.git/xen$ xenstore_client.o xenstore_client.c error: unknown warning option '-Wno-unknown-warning'; did you mean '-Wno-unknown-argument'? [-Werror,-Wunknown-warning-option] /local/xen.git/tools/xenstore/../../tools/Rules.mk:212: recipe for target 'xenstore_client.o' failed This definitely breaks the clang build, and a GCC workaround don't help. ~Andrew
>>> On 05.04.17 at 15:27, <ian.jackson@eu.citrix.com> wrote: > Andrew Cooper writes ("Re: [Xen-devel] [PATCH] tools/xenstore: Add > -Wno-bool-operation"): >> I'm fairly sure this will break the Clang build. > > This seems unlikely. I assume that clang has the same behaviour as > gcc, for -Wno-unknown-warning. I'm afraid this also breaks for older gcc (just tried 4.3.4), which we still consider supported. You need to probe the compiler via cc-option-add, just like done in ./Config.mk. Jan
diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile index 773d646..3b0ff16 100644 --- a/tools/xenstore/Makefile +++ b/tools/xenstore/Makefile @@ -5,6 +5,7 @@ MAJOR = 3.0 MINOR = 3 CFLAGS += -Werror +CFLAGS += -Wno-bool-operation CFLAGS += -I. # Include configure output (config.h) CFLAGS += -include $(XEN_ROOT)/tools/config.h
This suppresses a warning from GCC7 xenstored_domain.c:949:32: error: increment of a boolean expression We have not yet 100% concluded that this should not be fixed by changing the `_Bool b; b++;' to `b = 1'. But this fixes the warning and thus the build for now, and we want to apply the XSA-206 patches to stable trees. In the stable trees, this should be applied _before_ the XSA-206 series. Reported-by: Michael Young <m.a.young@durham.ac.uk> CC: Wei Liu <wei.liu2@citrix.com> CC: Jan Beulich <JBeulich@suse.com> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> --- tools/xenstore/Makefile | 1 + 1 file changed, 1 insertion(+)