Re: [PATCH] rust: str: add to_ascii_{upper,lower}case() to CString

[Danilo Krummrich]

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@xxxxxxxxxx> 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
>