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

Date:   Tue, 26 Sep 2017 17:36:56 +0900
From:   박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@....com) 
        <byungchul.park@....com>
To:     "Huang, Ying" <ying.huang@...el.com>,
        박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@....com) 
        <byungchul.park@....com>
CC:     "peterz@...radead.org" <peterz@...radead.org>,
        "mingo@...nel.org" <mingo@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "kernel-team@....com" <kernel-team@....com>
Subject: RE: [PATCH] llist: Put parentheses around parameters of
 llist_for_each_entry_safe()

> -----Original Message-----
> From: Huang, Ying [mailto:ying.huang@...el.com]
> Sent: Tuesday, September 26, 2017 5:14 PM
> To: 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@....com)
> Cc: Huang, Ying; peterz@...radead.org; mingo@...nel.org; linux-
> kernel@...r.kernel.org; kernel-team@....com
> Subject: Re: [PATCH] llist: Put parentheses around parameters of
> llist_for_each_entry_safe()
> 
> "박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@....com)"
> <byungchul.park@....com> writes:
> 
> >> -----Original Message-----
> >> From: Huang, Ying [mailto:ying.huang@...el.com]
> >> Sent: Tuesday, September 26, 2017 4:02 PM
> >> To: Byungchul Park
> >> Cc: peterz@...radead.org; mingo@...nel.org; 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()
> >>
> >> Hi, Byungchul,
> >>
> >> Byungchul Park <byungchul.park@....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@....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.
> 
> Yes, it's simple.  But I don't think it helps too.  Considering that
> list family functions with same style have no issues.

In addition, making all the llist macros implementation consistent is
also meaningful, isn't it?

Ok.

I think your opinion is right and mine is also meaningful. I will just
follow the majority opinion.

Thank you for giving an opinion.