RE: [PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe()

[박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com)]

> -----Original Message-----
> From: Huang, Ying [mailto:ying.huang@xxxxxxxxx]
> Sent: Tuesday, September 26, 2017 4:02 PM
> To: Byungchul Park
> Cc: peterz@xxxxxxxxxxxxx; mingo@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> kernel-team@xxxxxxx; ying.huang@xxxxxxxxx
> Subject: Re: [PATCH] llist: Put parentheses around parameters of
> llist_for_each_entry_safe()
> 
> Hi, Byungchul,
> 
> Byungchul Park <byungchul.park@xxxxxxx> writes:
> 
> > It would be somewhat safer to put parentheses around parameters of
> > a macro with parameters. Put it.
> >
> > Signed-off-by: Byungchul Park <byungchul.park@xxxxxxx>
> > ---
> >  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
> 
> The original code follows the style of list_for_each_entry_safe().  The

Hello Huang,

I don’t see what you say here exactly, but let me note that all llist macros
are safe except the llist_for_each_entry_safe().

> parameters "pos" and "n" must be variable.  Because list_xxx family
> functions work well so far, I think we needn't to change it too.

I see. I don't want to argue much wrt such a trivial thing but I think
it would be better to fix it since the fix is fairly simple and clear. 
However, it's ok if the fix introduces a bad thing at least.

> Best Regards,
> Huang, Ying