linux-kernel - Re: [PATCH] llist: Put parentheses around parameters of llist_for_each_entry

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] [day] [month] [year] [list]

Message-ID: <20171117023102.GA939@X58A-UD3R>
Date:   Fri, 17 Nov 2017 11:31:02 +0900
From:   Byungchul Park <byungchul.park@....com>
To:     peterz@...radead.org, mingo@...nel.org
Cc:     linux-kernel@...r.kernel.org, kernel-team@....com,
        ying.huang@...el.com
Subject: Re: [PATCH] llist: Put parentheses around parameters of
 llist_for_each_entry_safe()

On Tue, Sep 26, 2017 at 03:54:51PM +0900, Byungchul Park wrote:
> It would be somewhat safer to put parentheses around parameters of
> a macro with parameters. Put it.

Hello Ingo and Peter,

Even though Huang said this change is unnecessary, I'm curious about
what you think about this fix.

As Huang said, original code is safe unless users of llist use the
API weirdly, but I think this patch anyway makes the API safer and
align with the other llist APIs.

--
Thanks,
Byungchul

> Signed-off-by: Byungchul Park <byungchul.park@....com>
> ---
>  include/linux/llist.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/llist.h b/include/linux/llist.h
> index 1957635..e280b297 100644
> --- a/include/linux/llist.h
> +++ b/include/linux/llist.h
> @@ -183,10 +183,10 @@ static inline void init_llist_head(struct llist_head *list)
>   * reverse the order by yourself before traversing.
>   */
>  #define llist_for_each_entry_safe(pos, n, node, member)			       \
> -	for (pos = llist_entry((node), typeof(*pos), member);		       \
> +	for ((pos) = llist_entry((node), typeof(*(pos)), member);		       \
>  	     member_address_is_nonnull(pos, member) &&			       \
> -	        (n = llist_entry(pos->member.next, typeof(*n), member), true); \
> -	     pos = n)
> +	        ((n) = llist_entry((pos)->member.next, typeof(*(n)), member), true); \
> +	     (pos) = (n))
>  
>  /**
>   * llist_empty - tests whether a lock-less list is empty
> -- 
> 1.9.1