linux-kernel - Re: [PATCH] timekeeping: Use min() to fix Coccinelle warning

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 for Android: free password hash cracker in your pocket

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

Date: Tue, 25 Jun 2024 22:36:32 -0700
From: Thorsten Blum <thorsten.blum@...lux.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: jstultz@...gle.com,
 sboyd@...nel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] timekeeping: Use min() to fix Coccinelle warning

Hi Thomas,

On 24. Jun 2024, at 23:36, Thomas Gleixner <tglx@...utronix.de> wrote:
> On Mon, Jun 24 2024 at 08:24, Thorsten Blum wrote:
> 
>> Fixes the following Coccinelle/coccicheck warning reported by
>> minmax.cocci:
>> 
>> WARNING opportunity for min()
> 
> I'm fine with the change, but not so much with the change log.
> 
> You cannot fix a coccinelle warning. You can only fix the code which
> triggers the warning, right?
> 
> 'Opportunity to use min()' is nothing else than an opportunity, but
> what's the benefit of replacing correct code with it? What does this
> fix?
> 
> It fixes nothing. So calling it a fix is confusing at best.

I think it's pretty common to "fix a warning" -- there are thousands of
commits in the kernel using this wording in the summary alone -- even
when the change doesn't actually "fix" anything other than removing the
warning.

However, how about 'resolve' instead?

 timekeeping: Use min() to resolve Coccinelle warning

> What you want to say is something like this:
> 
> Subject: timekeeping: Replace open coded min()
> 
> Replace open coded min() because $GOOD_REASON
> 
> Discovered by minmax.cocci
> 
> $GOOD_REASON is not 'coccinelle emitted a warning'.

Removing a warning can be a good reason in itself to refactor code,
because fewer warnings make "real" warnings and potential problems
become more noticeable and thus more likely to get fixed. In short, it
improves maintainability.

To me this is obvious, but I'm happy to add something like "refactor
code to remove warning and improve overall maintainability" to the
commit message.

Thanks,
Thorsten