Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1739658

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

Show all headers | View raw

> -----Original Message-----
> From: Huang, Ying [mailto:ying.huang@intel.com]
> Sent: Tuesday, September 26, 2017 4:02 PM
> To: Byungchul Park
> Cc: peterz@infradead.org; mingo@kernel.org; linux-kernel@vger.kernel.org;
> kernel-team@lge.com; ying.huang@intel.com
> Subject: Re: [PATCH] llist: Put parentheses around parameters of
> llist_for_each_entry_safe()
> 
> Hi, Byungchul,
> 
> Byungchul Park <byungchul.park@lge.com> 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@lge.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
> 
> 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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe() Byungchul Park <byungchul.park@lge.com> - 2017-09-26 09:00 +0200
  Re: [PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe() "Huang\, Ying" <ying.huang@intel.com> - 2017-09-26 09:10 +0200
    RE: [PATCH] llist: Put parentheses around parameters of  llist_for_each_entry_safe() 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com)          <byungchul.park@lge.com> - 2017-09-26 10:10 +0200
      Re: [PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe() "Huang\, Ying" <ying.huang@intel.com> - 2017-09-26 10:20 +0200
        RE: [PATCH] llist: Put parentheses around parameters of  llist_for_each_entry_safe() 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com)          <byungchul.park@lge.com> - 2017-09-26 10:40 +0200

csiph-web