On 15/07/2019 15:19, Jan Beulich wrote: > On 15.07.2019 15:49, Andrew Cooper wrote: >> On 15/07/2019 11:35, 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 <jbeul...@suse.com> >>> --- >>> I'm open to going the !! or yet some different route. 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. >>> >>> --- 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); >> First and foremost, there is an identical construct on the ARM side, >> which wants an equivalent adjustment. > Oh, indeed. I should have remembered ... > >> As to the change, I'm going to suggest what I suggested instead of this >> the first time around, which is strlen(CONFIG_DOM0_MEM) to make the >> logic easier to follow. > How does use of strlen() make anything easier? I think it is a pretty > common pattern to check the first character for nul if all one is > after is to know whether a string is empty.
Only for things which are obviously a string. CONFIG_DOM0_MEM isn't. In this case, you have a name which would most obviously be an integer, which is compiling in a context where an array reference is valid, which is confusing to read. As strlen() is implemented with a builtin, it should be evaluated by the compiler, given that CONFIG_DOM0_MEM is a constant, but hopefully won't trigger this warning. ~Andrew _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
