linux-hardening - Re: [PATCH] lib/string_choices: Add str

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 15 Feb 2024 16:37:18 +0100
From: Michal Wajdeczko <michal.wajdeczko@...el.com>
To: Jani Nikula <jani.nikula@...el.com>, Kees Cook <keescook@...omium.org>,
 Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: linux-hardening@...r.kernel.org
Subject: Re: [PATCH] lib/string_choices: Add str_plural() helper



On 15.02.2024 16:20, Jani Nikula wrote:
> On Wed, 14 Feb 2024, Kees Cook <keescook@...omium.org> wrote:
>> On Wed, Feb 14, 2024 at 08:49:39PM +0200, Andy Shevchenko wrote:
>>> On Wed, Feb 14, 2024 at 08:30:53PM +0200, Jani Nikula wrote:
>>>> On Wed, 14 Feb 2024, Andy Shevchenko <andriy.shevchenko@...ux.intel.com> wrote:
>>>>> On Wed, Feb 14, 2024 at 08:08:16PM +0200, Andy Shevchenko wrote:
>>>>>> On Wed, Feb 14, 2024 at 05:50:15PM +0100, Michal Wajdeczko wrote:
>>>>>>> Add str_plural() helper to replace existing open implementations
>>>>>>> used by many drivers and help improve future user facing messages.
>>>>>>
>>>>>> Any user of this, please?
>>>>
>>>> git grep "\"\" *: *\"s\""
>>>
>>> You know what I meant.
>>> Second patch in the series that shows usefulness of the first patch.
>>
>> Or a Coccinelle script that could do some rewrites? But, yes, if you can
>> include some examples, that would be nice. I think the helper is fine to
>> add.

please find below requested Coccinelle script:

virtual patch
virtual context
virtual report

@depends on patch@
expression E;
@@
(
-	E == 1 ? "" : "s"
+	str_plural(E)
|
-	(E == 1) ? "" : "s"
+	str_plural(E)
|
-	E > 1 ? "s" : ""
+	str_plural(E)
|
-	(E > 1) ? "s" : ""
+	str_plural(E)
)

@r depends on !patch exists@
expression E;
position P;
@@
(
*	E@P == 1 ? "" : "s"
|
*	(E@P == 1) ? "" : "s"
|
*	E@P > 1 ? "s" : ""
|
*	(E@P > 1) ? "s" : ""
)

@script:python depends on report@
p << r.P;
e << r.E;
@@

coccilib.report.print_report(p[0], "opportunity for str_plural(%s)" % e)


> 
> In all kindness, I think it's actually better to get the inevitable
> bikeshedding rounds done first. It's trivial to add users after
> that. And for simple things like this it should really be obvious to
> everyone what the usage would be like.
> 
> My first attempt at adding a plural helper seems to date back to
> 2019. 

true, I do recall that, hence your invite

> Yes, really. Personally, I honestly can no longer be bothered to
> put in the extra effort before there's some indication of
> acceptance. (Like there's here now, thanks!)
> 
>>>>>>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@...el.com>
>>>>>>
>>>>>>> Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
>>>>>>> Cc: Jani Nikula <jani.nikula@...el.com>
>>>>>>
>>>>>> Move these Cc to the after '---' line.
>>>>
>>>> Why?
>>>
>>> At bare minimum to reduce noise in the commit message.
>>> If going further, to be environment friendly (no jokes).
>>
>> I learned the above-the-line CC habit from akpm. But in looking through
>> recent commit history, this does appear to be the exception now. I'll
>> need to adjust my own workflow for this too...
> 
> It just depends on where in the kernel you look, every subsystem is
> subtly different. In the drm subsystem if you added the Cc's below
> "---", you might be asked to move them above.
> 
> Trivial for whoever applies the patches to do whatever they please with
> the trailers, impossible for the contributor. It's like The Trial. :p
> 
> BR,
> Jani.
> 
>