Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Huang, Ying" <ying.huang@intel.com>
To:     Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>
Subject: Re: [RFC PATCH 4/4] llist.h: Fix parentheses around macro pointer
 parameter use
References: <20230504012914.1797355-1-mathieu.desnoyers@efficios.com>
        <20230504012914.1797355-4-mathieu.desnoyers@efficios.com>
Date:   Thu, 04 May 2023 13:54:14 +0800
In-Reply-To: <20230504012914.1797355-4-mathieu.desnoyers@efficios.com>
        (Mathieu Desnoyers's message of "Wed, 3 May 2023 21:29:14 -0400")
Message-ID: <87pm7gd4l5.fsf@yhuang6-desk2.ccr.corp.intel.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=ascii
Precedence: bulk

Hi, Mathieu,

Mathieu Desnoyers <mathieu.desnoyers@efficios.com> writes:

> Add missing parentheses around use of macro argument "pos" in those
> patterns to ensure operator precedence behaves as expected:
>
> - typeof(*pos)
> - pos->member
>
> The typeof(*pos) lack of parentheses around "pos" is not an issue per se
> in the specific macros modified here because "pos" is used as an lvalue,
> which should prevent use of any operator causing issue. Still add the
> extra parentheses for consistency.

I don't think it's necessary to add parentheses here.  As you said,
"pos" is used as an lvalue.

> Remove useless parentheses around use of macro parameter (node) in the
> following pattern:
>
>   llist_entry((node), typeof(*pos), member)
>
> Because comma is the lowest priority operator already, so the extra pair
> of parentheses is redundant.

This change looks good for me.

Best Regards,
Huang, Ying

> Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Huang Ying <ying.huang@intel.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> ---
>  include/linux/llist.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/llist.h b/include/linux/llist.h
> index 85bda2d02d65..45d358c15d0d 100644
> --- a/include/linux/llist.h
> +++ b/include/linux/llist.h
> @@ -173,9 +173,9 @@ 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); \
> +		(n = llist_entry((pos)->member.next, typeof(*(n)), member), true); \
>  	     pos = n)
>  
>  /**