linux-kernel - Re: [PATCH v2] locking/local_lock: s/l/__l/ and s/tl/_

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: <e9fa7e27-d622-4a1e-96c9-c819f5486619@suse.cz>
Date: Mon, 29 Sep 2025 09:16:47 +0200
From: Vlastimil Babka <vbabka@...e.cz>
To: Vincent Mailhol <mailhol@...nel.org>,
 Shakeel Butt <shakeel.butt@...ux.dev>,
 Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
 Alexei Starovoitov <ast@...nel.org>,
 Andrew Morton <akpm@...ux-foundation.org>,
 Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Will Deacon <will@...nel.org>, Waiman Long <longman@...hat.com>,
 Boqun Feng <boqun.feng@...il.com>
Cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] locking/local_lock: s/l/__l/ and s/tl/__tl/ to reduce
 risk of shadowing

On 9/27/25 05:04, Vincent Mailhol wrote:
> On 9/26/25 11:16 PM, Vlastimil Babka wrote:
>> +CC LOCKING PRIMITIVES maintainers. Looks like local_lock files were never
>> added to the section, should we?
>> 
>> On 9/24/25 20:03, Vincent Mailhol wrote:
>>> The Linux kernel coding style [1] advises to avoid common variable
>>> names in function-like macros to reduce the risk of collisions.
>> 
>> I think it would be better if the tools like sparse could recognize if the
>> shadowing happens inside a macro only and thus really unlikely to cause a
>> misuse due to confusion (code thinks it's manipulating an outer instance but
>> instead it's the inner one), because macros in their definition would never
>> intend to manipulate a possible outer instance, right? Or are there any
>> other problems due to shadowing besides this risk?
> 
> Thank would mean:
> 
>   - rewriting the shadowing check in sparse
>   - removing the -Wshadow from the W=2 list
>   - modifying the kernel coding style
> 
> I am not against this. But I am not unhappy with the current status quo either.
> 
> So far, I kept sending patches whenever I saw such shadow warning in header
> files. And over the last five years, this resulted in only three occurrences:
> 
>   - commit 146034fed6ee ("x86/asm/bitops: Use __builtin_ffs() to evaluate
>     constant expressions")
>     Link: https://git.kernel.org/torvalds/c/146034fed6ee
> 
> 
>   - commit 9ce02f0fc683 ("x86/bug: Prevent shadowing in __WARN_FLAGS")
>     Link: https://git.kernel.org/torvalds/c/9ce02f0fc683
> 
>   - this patch
> 
> Between sending one patch every couple year or enrolling to a quest to modify
> the tooling, my choice is already made. If someone else want to do this change,
> I would be supportive, but that person will not be me.

Thanks for that perspective, with that it seems now clear to me that a rare
fixup of some macro is indeed much easier.

> On a side note, I want to highlight that it is not that I am reluctant to modify
> the tooling. For example, I sent contributed this commit to sparse last week:
> 
> commit 366ad4b2fa3e ("Warn about "unsigned value that used to be signed against
> zero"")
> 
> Link:
> https://git.kernel.org/pub/scm/devel/sparse/sparse-dev.git/commit/?id=366ad4b2fa3e