From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S967957AbdIZIOe (ORCPT ); Tue, 26 Sep 2017 04:14:34 -0400 Received: from mga07.intel.com ([134.134.136.100]:32109 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753933AbdIZIOT (ORCPT ); Tue, 26 Sep 2017 04:14:19 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,440,1500966000"; d="scan'208";a="1199070154" From: "Huang\, Ying" To: =?utf-8?B?67CV67OR7LKgL+yEoOyehOyXsOq1rOybkC9TVyBQbGF0Zm9ybSjsl7ApQU9U?= =?utf-8?B?7YyAKGJ5dW5nY2h1bC5wYXJrQGxnZS5jb20p?= 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() References: <1506408891-27460-1-git-send-email-byungchul.park@lge.com> <8760c6c59e.fsf@yhuang-dev.intel.com> Date: Tue, 26 Sep 2017 16:14:16 +0800 In-Reply-To: (=?utf-8?B?IuuwleuzkeyyoCIncw==?= message of "Tue, 26 Sep 2017 17:01:50 +0900") Message-ID: <871smtdgh3.fsf@yhuang-dev.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org "박병철/선임연구원/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. Best Regards, Huang, Ying