Re: [PATCH] rust: str: add conversion from `CStr` to `CString`

[Alice Ryhl]

On 5/2/23 20:02, Benno Lossin wrote:
> On 02.05.23 18:59, Wedson Almeida Filho wrote:
> > On Tue, 2 May 2023 at 09:53, Alice Ryhl <aliceryhl@xxxxxxxxxx> wrote:
> > > +impl<'a> TryFrom<&'a CStr> for CString {
> > > +    type Error = TryReserveError;
> >
> > Wouldn't `AllocError` make more sense? Or even Error (with ENOMEM value).
> >
> > `TryReserveError` is documented as "The error type for try_reserve
> > methods." -- that fact the we use a `Vec` is an implementation detail,
> > I feel it's better not to leak this fact through the public API.
>
> I agree, it should be `AllocError`. There is a `From<AllocError> for Error`
> with `ENOMEM` as the value, so `AllocError` is the most compatible, since it
> simply converts to `Error` via `?`.

Sounds good to me.

> Technically, `TryReserveError` represents two different kinds of errors:
> - CapacityOverflow -- triggered when exceeding `isize::MAX` bytes of size
> - AllocError -- memory allocation failed
>
> I think it is fine to coalesce these into `AllocError`, since allocating
> `isize::MAX` might as well be considered an OOM error.
In fact, the `isize::MAX` case is unreachable since that would require 
you to already have a `&CStr` of that size, which Rust does not allow.

> With that fixed:
> Reviewed-by: Benno Lossin <benno.lossin@xxxxxxxxx>

Thanks both of you. I'll submit a v2 tomorrow.

Alice