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;
Message-ID: <532c91c5-5655-2a15-8bf1-82ee939c156b@efficios.com>
Date:   Thu, 4 May 2023 10:45:18 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [RFC PATCH 2/4] list.h: Fix parentheses around macro pointer
 parameter use
Content-Language: en-US
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        linux-kernel@vger.kernel.org,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        David Howells <dhowells@redhat.com>,
        Ricardo Martinez <ricardo.martinez@linux.intel.com>
References: <20230504012914.1797355-1-mathieu.desnoyers@efficios.com>
 <20230504012914.1797355-2-mathieu.desnoyers@efficios.com>
 <ZFPEEVvHTw2uukun@smile.fi.intel.com>
From:   Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
In-Reply-To: <ZFPEEVvHTw2uukun@smile.fi.intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2023-05-04 10:41, Andy Shevchenko wrote:
> On Wed, May 03, 2023 at 09:29:12PM -0400, Mathieu Desnoyers wrote:
>> Add missing parentheses around use of macro argument "pos" in those
>> patterns to ensure operator precedence behaves as expected:
>>
>> - typeof(*pos)
>> - pos->member
>>
>> Remove useless parentheses around use of macro parameter (head) in the
>> following pattern:
>>
>> - list_is_head(pos, (head))
>>
>> Because comma is the lowest priority operator already, so the extra pair
>> of parentheses is redundant.
>>
>> This corrects the following usage pattern where operator precedence is
>> unexpected:
>>
>>    LIST_HEAD(testlist);
>>
>>    struct test {
>>            struct list_head node;
>>            int a;
>>    };
>>
>>    // pos->member issue
>>    void f(void)
>>    {
>>            struct test *t1;
>>            struct test **t2 = &t1;
> 
> I'm not against the patch, but I'm in doubt, looking into this example, it's useful.
> Any real use case like above in the Linux kernel, please?

There aren't because the code would not compile with the current header 
implementation. But it's unexpected that this kind of pattern does not work.

It's not about being useful, but rather about eliminating unexpected 
operator precedence within macros, and about being consistent everywhere.

Thanks,

Mathieu

> 
>>            list_for_each_entry((*t2), &testlist, node) {   /* works */
>>                    //...
>>            }
>>            list_for_each_entry(*t2, &testlist, node) {     /* broken */
>>                    //...
>>            }
>>    }
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com