linux-kernel - Re: [RFC PATCH 2/4] list.h: Fix parentheses around macro pointer parameter use

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <532c91c5-5655-2a15-8bf1-82ee939c156b@efficios.com>
Date:   Thu, 4 May 2023 10:45:18 -0400
From:   Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        David Howells <dhowells@...hat.com>,
        Ricardo Martinez <ricardo.martinez@...ux.intel.com>
Subject: Re: [RFC PATCH 2/4] list.h: Fix parentheses around macro pointer
 parameter use

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