On Tue, May 05, 2020 at 03:47:43PM +0200, Jan Beulich wrote:
> [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments
> unless you have verified the sender and know the content is safe.
>
> On 05.05.2020 11:24, Roger Pau Monne wrote:
> > Clang 10 complains with:
> >
> > mm.c:1239:10: error: converting the result of '<<' to a boolean always
> > evaluates to true
> > [-Werror,-Wtautological-constant-compare]
> > if ( _PAGE_GNTTAB && (l1e_get_flags(l1e) & _PAGE_GNTTAB) &&
> > ^
> > xen/include/asm/x86_64/page.h:161:25: note: expanded from macro
> > '_PAGE_GNTTAB'
> > #define _PAGE_GNTTAB (1U<<22)
> > ^
>
> This is a rather odd warning. Do they also warn for "if ( 0 )"
> or "do { } while ( 0 )", as we use in various places? There's
> no difference to me between a plain number and a constant
> composed via an expression.
Using plain 0 is fine, they just seem to dislike using << for some
reason that escapes me. Seems like it might be useful to catch bugs
where || is wrongly used instead of | when setting flags, ie:
https://github.com/haproxy/haproxy/issues/588
>
> > Remove the conversion of _PAGE_GNTTAB to a boolean, since the and
> > operation performed afterwards will already return false if the value
> > of the macro is 0.
>
> I'm sorry, but no. The expression was put there on purpose by
> 0932210ac095 ("x86: Address "Bitwise-and with zero
> CONSTANT_EXPRESSION_RESULT" Coverity issues"), and the
> description there is clearly telling us that this wants to stay
> unless Coverity changed in the meantime. Otherwise I'm afraid
> a more elaborate solution will be needed to please both.
Clang is fine with changing this to _PAGE_GNTTAB != 0. Would you be
OK with this approach?
> Or a
> more simplistic one, like using "#if _PAGE_GNTTAB" around the
> construct.
Yes, that's the other solution I had in mind.
Thanks, Roger.
