linux-kernel - Re: [PATCH] rust: str: add to_ascii

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: <Za7pRc1wTtEhObTN@cassiopeiae>
Date: Mon, 22 Jan 2024 23:16:37 +0100
From: Danilo Krummrich <dakr@...hat.com>
To: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc: ojeda@...nel.org, alex.gaynor@...il.com, wedsonaf@...il.com,
	boqun.feng@...il.com, gary@...yguo.net, bjorn3_gh@...tonmail.com,
	benno.lossin@...ton.me, a.hindborg@...sung.com,
	aliceryhl@...gle.com, rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rust: str: add to_ascii_{upper,lower}case() to CString

On Mon, Jan 22, 2024 at 08:38:26PM +0100, Miguel Ojeda wrote:
> On Mon, Jan 22, 2024 at 7:46 PM Danilo Krummrich <dakr@...hat.com> wrote:
> >
> > Add functions to convert a CString to upper- / lowercase assuming all
> > characters are ASCII encoded.
> 
> Like Alice mentioned, please mention the use case, i.e. the "why?"

Sure, I need this in the context of converting stringified enum values
(representing different GPU chipsets) to strings in order to generate the
corresponding firmware paths. The project context is Nova (GSP only Rust
successor of Nouveau).

If preferred, I can add this to the commit message.

> (perhaps also linking the Zulip discussion if you like [1]).
> 
> [1] https://rust-for-linux.zulipchat.com/#narrow/stream/288089-General/topic/String.20manipulation.20in.20kernel.20Rust

Sure, gonna add it.

> 
> > +    /// Converts the whole CString to lowercase.
> 
> Please use Markdown and, if possible, an intra-doc link, i.e.
> 
>     /// Converts the whole [`CString`] to lowercase.
> 
> Also perhaps we should mimic the standard library docs?

Mimic, as in copy them over (to the extent they actually apply)?

> 
> > +    pub fn to_ascii_lowercase(&mut self) {
> > +        self.buf.make_ascii_lowercase();
> > +    }
> 
> Why did you choose the `to_ascii_*()` name for these? In the standard
> library, the in-place ones are `make_ascii_*()` (like the one you call
> in the implementation).

Should be make_ascii_*(), agreed.

> 
> Should the new-object-returned ones be added, by the way, if we are
> adding these?

Sure, I'm fine adding them as well. Not sure we'll need them for Nova though.

- Danilo

> 
> Cheers,
> Miguel
>