Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 16 May 2023 11:12:02 +0000
In-Reply-To: <871qjhe9xh.fsf@metaspace.dk>
Mime-Version: 1.0
References: <871qjhe9xh.fsf@metaspace.dk>
Message-ID: <20230516111202.2455529-1-aliceryhl@google.com>
Subject: Re: [PATCH v2] rust: str: add conversion from `CStr` to `CString`
From:   Alice Ryhl <aliceryhl@google.com>
To:     nmi@metaspace.dk
Cc:     alex.gaynor@gmail.com, alice@ryhl.io, benno.lossin@proton.me,
        bjorn3_gh@protonmail.com, boqun.feng@gmail.com, gary@garyguo.net,
        linux-kernel@vger.kernel.org, ojeda@kernel.org,
        patches@lists.linux.dev, rust-for-linux@vger.kernel.org,
        wedsonaf@gmail.com, yakoyoku@gmail.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Andreas Hindborg <nmi@metaspace.dk> writes:
> Alice Ryhl <alice@ryhl.io> writes:
>> On 5/8/23 13:41, Gary Guo wrote:
>>> On Wed,  3 May 2023 14:10:16 +0000
>>> Alice Ryhl <aliceryhl@google.com> wrote:
>>>> These methods can be used to copy the data in a temporary c string into
>>>> a separate allocation, so that it can be accessed later even if the
>>>> original is deallocated.
>>>>
>>>> The API in this change mirrors the standard library API for the `&str`
>>>> and `String` types. The `ToOwned` trait is not implemented because it
>>>> assumes that allocations are infallible.
>>> How about add a `TryToOwned` trait to the kernel crate and implement
>>> that trait for `CStr` instead?
>>
>> Eh, I don't think it's worth it. It doesn't give anything new to the CStr api,
>> and I think it's rather unlikely that someone will actually need to be generic
>> over such a trait any time soon.
> 
> It is just as valid as having `From<&str>` and `ToOwned<&str>`. While it
> does not add anything in terms of function, it carries intention. I
> think we should consider adding it at some point.
> 
> BR Andreas

Sure, I think its quite reasonable to add new traits, I just don't think
it should be part of this patch. Adding new traits makes it a
significantly bigger change IMO, and my changes have an actual user down
the road.

Alice