Re: [PATCH] mm: fix nodemask printing

[Michal Hocko]

On Fri 17-11-17 11:15:45, Arnd Bergmann wrote:
> The cleanup caused build warnings for constant mask pointers:
> 
> mm/mempolicy.c: In function âmpol_to_strâ:
> ./include/linux/nodemask.h:108:11: warning: the comparison will always evaluate as âtrueâ for the address of ânodesâ will never be NULL [-Waddress]
> 
> An earlier workaround I suggested was incorporated in the version that
> got merged, but that only solved the problem for gcc-7 and higher, while
> gcc-4.6 through gcc-6.x still warn.
> 
> This changes the printing again to use inline functions that make it
> clear to the compiler that the line that does the NULL check has no
> idea whether the argument is a constant NULL.
> 
> Fixes: 0205f75571e3 ("mm: simplify nodemask printing")
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>

Thanks for the fixup. It is sad how a questionable warning makes the
code worse... Does it make sense to have the warning enabled?
Gcc bug  [1] suggests there is no great interest into fixing it.

Anyway to the patch
Acked-by: Michal Hocko <mhocko@xxxxxxxx>

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82963

> ---
> I've done only minimal build testing on this, but did check it with
> all compiler versions this time.
> ---
>  include/linux/nodemask.h | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/nodemask.h b/include/linux/nodemask.h
> index 15cab3967d6d..1fbde8a880d9 100644
> --- a/include/linux/nodemask.h
> +++ b/include/linux/nodemask.h
> @@ -104,9 +104,16 @@ extern nodemask_t _unused_nodemask_arg_;
>   *
>   * Can be used to provide arguments for '%*pb[l]' when printing a nodemask.
>   */
> -#define nodemask_pr_args(maskp)				\
> -	((maskp) != NULL) ? MAX_NUMNODES : 0,		\
> -	((maskp) != NULL) ? (maskp)->bits : NULL
> +#define nodemask_pr_args(maskp)	__nodemask_pr_numnodes(maskp), \
> +				__nodemask_pr_bits(maskp)
> +static inline unsigned int __nodemask_pr_numnodes(const nodemask_t *m)
> +{
> +	return m ? MAX_NUMNODES : 0;
> +}
> +static inline const unsigned long *__nodemask_pr_bits(const nodemask_t *m)
> +{
> +	return m ? m->bits : NULL;
> +}
>  
>  /*
>   * The inline keyword gives the compiler room to decide to inline, or
> -- 
> 2.9.0

-- 
Michal Hocko
SUSE Labs