> -----Original Message----- > From: Huang, Ying [mailto:ying.hu...@intel.com] > Sent: Tuesday, September 26, 2017 5:14 PM > To: 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.p...@lge.com) > Cc: Huang, Ying; pet...@infradead.org; mi...@kernel.org; linux- > ker...@vger.kernel.org; kernel-t...@lge.com > Subject: Re: [PATCH] llist: Put parentheses around parameters of > llist_for_each_entry_safe() > > "박병철/선임연구원/SW Platform(연)AOT팀(byungchul.p...@lge.com)" > <byungchul.p...@lge.com> writes: > > >> -----Original Message----- > >> From: Huang, Ying [mailto:ying.hu...@intel.com] > >> Sent: Tuesday, September 26, 2017 4:02 PM > >> To: Byungchul Park > >> Cc: pet...@infradead.org; mi...@kernel.org; linux-kernel@vger.kernel.org; > >> kernel-t...@lge.com; ying.hu...@intel.com > >> Subject: Re: [PATCH] llist: Put parentheses around parameters of > >> llist_for_each_entry_safe() > >> > >> Hi, Byungchul, > >> > >> Byungchul Park <byungchul.p...@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.p...@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. > > 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.
