[v2] dom0-build: fix build with clang5
diff mbox series

Message ID 5c88239b-de0f-5f81-72c4-7fdb07524278@suse.com
State New
Headers show
Series
  • [v2] dom0-build: fix build with clang5
Related show

Commit Message

Jan Beulich July 17, 2019, 6:47 a.m. UTC
With non-empty CONFIG_DOM0_MEM clang5 produces

dom0_build.c:344:24: error: use of logical '&&' with constant operand [-Werror,-Wconstant-logical-operand]
     if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
                        ^  ~~~~~~~~~~~~~~~~~~
dom0_build.c:344:24: note: use '&' for a bitwise operation
     if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
                        ^~
                        &
dom0_build.c:344:24: note: remove constant to silence this warning
     if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
                       ~^~~~~~~~~~~~~~~~~~~~~
1 error generated.

Obviously neither of the two suggestions are an option here. Oddly
enough swapping the operands of the && helps, while e.g. casting or
parenthesizing doesn't. Another workable variant looks to be the use of
!! on the constant.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Also adjust the Arm incarnation of the same construct.
---
I'm open to going the !! or yet some different route (but not really the
suggested strlen() one). No matter which one we choose, I'm afraid it is
going to remain guesswork what newer (and future) versions of clang will
choke on.

Comments

Julien Grall July 29, 2019, 10:05 a.m. UTC | #1
Hi,

On 7/17/19 7:47 AM, Jan Beulich wrote:
> With non-empty CONFIG_DOM0_MEM clang5 produces
> 
> dom0_build.c:344:24: error: use of logical '&&' with constant operand [-Werror,-Wconstant-logical-operand]
>       if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>                          ^  ~~~~~~~~~~~~~~~~~~
> dom0_build.c:344:24: note: use '&' for a bitwise operation
>       if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>                          ^~
>                          &
> dom0_build.c:344:24: note: remove constant to silence this warning
>       if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>                         ~^~~~~~~~~~~~~~~~~~~~~
> 1 error generated.
> 
> Obviously neither of the two suggestions are an option here. Oddly
> enough swapping the operands of the && helps, while e.g. casting or
> parenthesizing doesn't. Another workable variant looks to be the use of
> !! on the constant.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Also adjust the Arm incarnation of the same construct.
> ---
> I'm open to going the !! or yet some different route (but not really the
> suggested strlen() one). No matter which one we choose, I'm afraid it is
> going to remain guesswork what newer (and future) versions of clang will
> choke on.

I quite like the strlen one, however looking around online this may not 
solve the problem. AFAIK, Clang is not happy because the constant is not 
a boolean.

So !! or != 0 should work here.

Cheers,
Jan Beulich July 29, 2019, 11:36 a.m. UTC | #2
On 29.07.2019 12:05, Julien Grall wrote:
> Hi,
> 
> On 7/17/19 7:47 AM, Jan Beulich wrote:
>> With non-empty CONFIG_DOM0_MEM clang5 produces
>>
>> dom0_build.c:344:24: error: use of logical '&&' with constant operand [-Werror,-Wconstant-logical-operand]
>>       if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>>                          ^  ~~~~~~~~~~~~~~~~~~
>> dom0_build.c:344:24: note: use '&' for a bitwise operation
>>       if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>>                          ^~
>>                          &
>> dom0_build.c:344:24: note: remove constant to silence this warning
>>       if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
>>                         ~^~~~~~~~~~~~~~~~~~~~~
>> 1 error generated.
>>
>> Obviously neither of the two suggestions are an option here. Oddly
>> enough swapping the operands of the && helps, while e.g. casting or
>> parenthesizing doesn't. Another workable variant looks to be the use of
>> !! on the constant.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> v2: Also adjust the Arm incarnation of the same construct.
>> ---
>> I'm open to going the !! or yet some different route (but not really the
>> suggested strlen() one). No matter which one we choose, I'm afraid it is
>> going to remain guesswork what newer (and future) versions of clang will
>> choke on.
> 
> I quite like the strlen one, however looking around online this may not
> solve the problem. AFAIK, Clang is not happy because the constant is not
> a boolean.

I don't think it's as simple as "not a boolean", but I also haven't
played with it more than I had to in order to find possible
workarounds. As to the strlen() approach - I've expressed my dislike
before.

> So !! or != 0 should work here.

Well, with perhaps as much of a chance of breaking again as any
other of the possible solutions, unless they recognize and address
the oddity. Andrew - would you be willing to ack a v3 using !!
instead of re-ordering the && operands (seeing that you didn't ack
v2 yet, and perhaps you're also not meaning to)?

Jan

Patch
diff mbox series

--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2125,7 +2125,7 @@  int __init construct_dom0(struct domain
  
      printk("*** LOADING DOMAIN 0 ***\n");
  
-    if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
+    if ( CONFIG_DOM0_MEM[0] && !dom0_mem_set )
          parse_dom0_mem(CONFIG_DOM0_MEM);
  
      if ( dom0_mem <= 0 )
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -341,7 +341,7 @@  unsigned long __init dom0_compute_nr_pag
      unsigned long avail = 0, nr_pages, min_pages, max_pages;
      bool need_paging;
  
-    if ( !dom0_mem_set && CONFIG_DOM0_MEM[0] )
+    if ( CONFIG_DOM0_MEM[0] && !dom0_mem_set )
          parse_dom0_mem(CONFIG_DOM0_MEM);
  
      for_each_node_mask ( node, dom0_nodes )