Re: [Xen-devel] Ping: [PATCH v2] dom0-build: fix build with clang5

[Jan Beulich <jbeulich@suse.com>]

On 15.01.2020 11:00, Roger Pau Monné wrote:
> On Wed, Jan 15, 2020 at 10:56:37AM +0100, Roger Pau Monné wrote:
>> On Fri, Dec 20, 2019 at 05:26:34PM +0100, Jan Beulich wrote:
>>> On 17.07.2019 08:47, 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 guess the disagreement on how to exactly address the issue has
>>> stalled this. But I think we should rather have _some_ (e.g.
>>> this) solution in the repo, than continue to ship versions which
>>> don't build. People wanting to beautify the code further could
>>> then submit incremental patches.
>>
>> Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

>> I'm not providing a RB because this is all guesswork, so it doesn't
>> feel appropriate to review something that's based on undocumented
>> compiler behavior.
>>
>> Another option would be to pass -Wconstant-logical-operand but that
>> would prevent caching some licit issues.
> 
> Forgot to mention, but could you please add a comment to note that the
> condition is ordered this way to make clang5 happy?

I've added

    /* The ordering of operands is to work around a clang5 issue. */

to both instances.

Jan

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