linux-kernel - Re: [PATCH] mm: fix nodemask printing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 20 Nov 2017 09:22:39 +0100
From:   Michal Hocko <mhocko@...nel.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Stephen Rothwell <sfr@...b.auug.org.au>,
        linux-next@...r.kernel.org, linux-kernel@...r.kernel.org,
        Zhangshaokun <zhangshaokun@...ilicon.com>
Subject: Re: [PATCH] mm: fix nodemask printing

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@...db.de>

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@...e.com>

[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