lkml.org 
[lkml]   [2017]   [Sep]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe()
Date
"박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com)"
<byungchul.park@lge.com> writes:

>> -----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.

Yes, it's simple. But I don't think it helps too. Considering that
list family functions with same style have no issues.

Best Regards,
Huang, Ying

\
 
 \ /
  Last update: 2017-09-26 10:15    [W:0.108 / U:0.640 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site