From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S968169AbdIZIhD (ORCPT ); Tue, 26 Sep 2017 04:37:03 -0400 Received: from LGEAMRELO11.lge.com ([156.147.23.51]:33927 "EHLO lgeamrelo11.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964936AbdIZIg7 (ORCPT ); Tue, 26 Sep 2017 04:36:59 -0400 X-Original-SENDERIP: 156.147.1.121 X-Original-MAILFROM: byungchul.park@lge.com X-Original-SENDERIP: 165.244.98.76 X-Original-MAILFROM: byungchul.park@lge.com From: =?utf-8?B?67CV67OR7LKgL+yEoOyehOyXsOq1rOybkC9TVyBQbGF0Zm9ybSjsl7ApQU9U?= =?utf-8?B?7YyAKGJ5dW5nY2h1bC5wYXJrQGxnZS5jb20p?= To: "Huang, Ying" , =?utf-8?B?67CV67OR7LKgL+yEoOyehOyXsOq1rOybkC9TVyBQbGF0Zm9ybSjsl7ApQU9U?= =?utf-8?B?7YyAKGJ5dW5nY2h1bC5wYXJrQGxnZS5jb20p?= CC: "peterz@infradead.org" , "mingo@kernel.org" , "linux-kernel@vger.kernel.org" , "kernel-team@lge.com" Date: Tue, 26 Sep 2017 17:36:56 +0900 Subject: RE: [PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe() Thread-Topic: [PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe() Thread-Index: AdM2n4qCzI3335tsRNq5A/NdJat68AAAKfBg Message-ID: References: <1506408891-27460-1-git-send-email-byungchul.park@lge.com> <8760c6c59e.fsf@yhuang-dev.intel.com> <871smtdgh3.fsf@yhuang-dev.intel.com> In-Reply-To: <871smtdgh3.fsf@yhuang-dev.intel.com> Accept-Language: ko-KR, en-US Content-Language: ko-KR X-MS-Has-Attach: X-MS-TNEF-Correlator: acceptlanguage: ko-KR, en-US Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by nfs id v8Q8bh6U024931 > -----Original Message----- > From: Huang, Ying [mailto:ying.huang@intel.com] > Sent: Tuesday, September 26, 2017 5:14 PM > To: 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com) > Cc: Huang, Ying; peterz@infradead.org; mingo@kernel.org; linux- > kernel@vger.kernel.org; kernel-team@lge.com > Subject: Re: [PATCH] llist: Put parentheses around parameters of > llist_for_each_entry_safe() > > "박병철/선임연구원/SW Platform(연)AOT팀(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 writes: > >> > >> > It would be somewhat safer to put parentheses around parameters of > >> > a macro with parameters. Put it. > >> > > >> > Signed-off-by: Byungchul Park > >> > --- > >> > 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.