Re: [PATCH v4] Add methods for FromBytes trait.

[Benno Lossin]

On Tue Mar 18, 2025 at 12:14 AM CET, Christian wrote:
> Hi, Benno.
>
>> It usually is a good idea to include a changelog and a link to any prior
>> versions after this `---`. It won't be included in the final commit
>> message, but help reviewers and others keep track of this series.
>
> Yeah, my bad. I forgot.
>
>> I think this section should go before the `Safety` section.
>
> I followed this section:
> https://docs.kernel.org/rust/coding-guidelines.html#code-documentation,
> but no problem, I'll change.

Ah I see, I'll change that then.

>> Why is this trait becoming safe?
>
> I thought that if we change to a Result and get the Err case, it's not
> a problem to be safe.

A trait being `unsafe` means that the implementer needs to justify why
their implementation is correct. The fact that you change the return
type to `Result` doesn't change that the type must be transmutable from
sufficiently many bytes.

>> IMO it makes more sense for the return type to be `Option<&Self>`.
>
> I agree. I'll change.
>
>> This must also require that `Self: AsBytes`, since otherwise the user
>> could write padding bytes into the original slice.
>
> Did you mean `ToBytes`? Should I create another patch with an empty trait, e.g
> ```
> unsafe trait ToBytes {}
> ```
> or create the trait and its methods?

Nope, I mean `AsBytes`, it already exists in `rust/kernel/transmute.rs`.

>> Also the parameter name `mut_slice_of_bytes` is a bit long, how about
>> `bytes`?
>
> I liked it, I'll change to `bytes` and `bytes_mut`

I wouldn't put `_mut` in the parameter name, just name both of them
`bytes`.

>> What is this safety comment for?
>
> Idk if I should create another safety comment or just continue. In
> this case, I choose the first and submit the patch. So how should I
> proceed?

I don't understand, the safety comment that you added there doesn't make
any sense to me. I wouldn't have added it.

---
Cheers,
Benno