2024-03-13 23:10:18

by Benno Lossin

[permalink] [raw]
Subject: [PATCH] rust: init: remove impl Zeroable for Infallible

From: Laine Taffin Altman <[email protected]>

It is not enough for a type to be a ZST to guarantee that zeroed memory
is a valid value for it; it must also be inhabited. Creating a value of
an uninhabited type, ZST or no, is immediate UB.
Thus remove the implementation of `Zeroable` for `Infallible`, since
that type is not inhabited.

Cc: [email protected]
Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
Signed-off-by: Laine Taffin Altman <[email protected]>
Signed-off-by: Benno Lossin <[email protected]>
---
rust/kernel/init.rs | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
index 424257284d16..538e03cfc84a 100644
--- a/rust/kernel/init.rs
+++ b/rust/kernel/init.rs
@@ -1292,8 +1292,8 @@ macro_rules! impl_zeroable {
i8, i16, i32, i64, i128, isize,
f32, f64,

- // SAFETY: These are ZSTs, there is nothing to zero.
- {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, Infallible, (),
+ // SAFETY: These are inhabited ZSTs, there is nothing to zero and a valid value exists.
+ {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, (),

// SAFETY: Type is allowed to take any value, including all zeros.
{<T>} MaybeUninit<T>,

base-commit: 768409cff6cc89fe1194da880537a09857b6e4db
--
2.42.0




2024-03-14 09:15:50

by Alice Ryhl

[permalink] [raw]
Subject: Re: [PATCH] rust: init: remove impl Zeroable for Infallible

On Thu, Mar 14, 2024 at 12:09 AM Benno Lossin <[email protected]> wrote:
>
> From: Laine Taffin Altman <[email protected]>
>
> It is not enough for a type to be a ZST to guarantee that zeroed memory
> is a valid value for it; it must also be inhabited. Creating a value of
> an uninhabited type, ZST or no, is immediate UB.
> Thus remove the implementation of `Zeroable` for `Infallible`, since
> that type is not inhabited.
>
> Cc: [email protected]
> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
> Signed-off-by: Laine Taffin Altman <[email protected]>
> Signed-off-by: Benno Lossin <[email protected]>

Reviewed-by: Alice Ryhl <[email protected]>

2024-03-18 17:26:00

by Boqun Feng

[permalink] [raw]
Subject: Re: [PATCH] rust: init: remove impl Zeroable for Infallible

On Wed, Mar 13, 2024 at 11:09:37PM +0000, Benno Lossin wrote:
> From: Laine Taffin Altman <[email protected]>
>
> It is not enough for a type to be a ZST to guarantee that zeroed memory
> is a valid value for it; it must also be inhabited. Creating a value of
> an uninhabited type, ZST or no, is immediate UB.
> Thus remove the implementation of `Zeroable` for `Infallible`, since
> that type is not inhabited.
>
> Cc: [email protected]
> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
> Signed-off-by: Laine Taffin Altman <[email protected]>
> Signed-off-by: Benno Lossin <[email protected]>

I think either in the commit log or in the code comment, there better be
a link or explanation on "(un)inhabited type". The rest looks good to
me.

Reviewed-by: Boqun Feng <[email protected]>

Regards,
Boqun

> ---
> rust/kernel/init.rs | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> index 424257284d16..538e03cfc84a 100644
> --- a/rust/kernel/init.rs
> +++ b/rust/kernel/init.rs
> @@ -1292,8 +1292,8 @@ macro_rules! impl_zeroable {
> i8, i16, i32, i64, i128, isize,
> f32, f64,
>
> - // SAFETY: These are ZSTs, there is nothing to zero.
> - {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, Infallible, (),
> + // SAFETY: These are inhabited ZSTs, there is nothing to zero and a valid value exists.
> + {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, (),
>
> // SAFETY: Type is allowed to take any value, including all zeros.
> {<T>} MaybeUninit<T>,
>
> base-commit: 768409cff6cc89fe1194da880537a09857b6e4db
> --
> 2.42.0
>
>
>

2024-03-19 03:17:36

by Laine Taffin Altman

[permalink] [raw]
Subject: Re: [PATCH] rust: init: remove impl Zeroable for Infallible

On Mar 18, 2024, at 10:25 AM, Boqun Feng <[email protected]> wrote:
> On Wed, Mar 13, 2024 at 11:09:37PM +0000, Benno Lossin wrote:
>> From: Laine Taffin Altman <[email protected]>
>>
>> It is not enough for a type to be a ZST to guarantee that zeroed memory
>> is a valid value for it; it must also be inhabited. Creating a value of
>> an uninhabited type, ZST or no, is immediate UB.
>> Thus remove the implementation of `Zeroable` for `Infallible`, since
>> that type is not inhabited.
>>
>> Cc: [email protected]
>> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
>> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
>> Signed-off-by: Laine Taffin Altman <[email protected]>
>> Signed-off-by: Benno Lossin <[email protected]>
>
> I think either in the commit log or in the code comment, there better be
> a link or explanation on "(un)inhabited type". The rest looks good to
> me.

Would the following be okay for that purpose?

A type is inhabited if at least one valid value of that type exists; a
type is uninhabited if no valid values of that type exist. The terms
"inhabited" and "uninhabited" in this sense originate in type theory,
a branch of mathematics.

In Rust, producing an invalid value of any type is immediate undefined
behavior (UB); this includes via zeroing memory. Therefore, since an
uninhabited type has no valid values, producing any values at all for
it is UB.

The Rust standard library type `core::convert::Infallible` is
uninhabited, by virtue of having been declared as an enum with no
cases, which always produces uninhabited types in Rust. Thus, remove
the implementation of `Zeroable` for `Infallible`, thereby avoiding
the UB.

Thanks,
Laine

>
> Reviewed-by: Boqun Feng <[email protected]>
>
> Regards,
> Boqun
>
>> ---
>> rust/kernel/init.rs | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
>> index 424257284d16..538e03cfc84a 100644
>> --- a/rust/kernel/init.rs
>> +++ b/rust/kernel/init.rs
>> @@ -1292,8 +1292,8 @@ macro_rules! impl_zeroable {
>> i8, i16, i32, i64, i128, isize,
>> f32, f64,
>>
>> - // SAFETY: These are ZSTs, there is nothing to zero.
>> - {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, Infallible, (),
>> + // SAFETY: These are inhabited ZSTs, there is nothing to zero and a valid value exists.
>> + {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, (),
>>
>> // SAFETY: Type is allowed to take any value, including all zeros.
>> {<T>} MaybeUninit<T>,
>>
>> base-commit: 768409cff6cc89fe1194da880537a09857b6e4db
>> --
>> 2.42.0
>>
>>
>>


2024-03-19 04:39:37

by Boqun Feng

[permalink] [raw]
Subject: Re: [PATCH] rust: init: remove impl Zeroable for Infallible

On Mon, Mar 18, 2024 at 08:17:07PM -0700, Laine Taffin Altman wrote:
> On Mar 18, 2024, at 10:25 AM, Boqun Feng <[email protected]> wrote:
> > On Wed, Mar 13, 2024 at 11:09:37PM +0000, Benno Lossin wrote:
> >> From: Laine Taffin Altman <[email protected]>
> >>
> >> It is not enough for a type to be a ZST to guarantee that zeroed memory
> >> is a valid value for it; it must also be inhabited. Creating a value of
> >> an uninhabited type, ZST or no, is immediate UB.
> >> Thus remove the implementation of `Zeroable` for `Infallible`, since
> >> that type is not inhabited.
> >>
> >> Cc: [email protected]
> >> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
> >> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
> >> Signed-off-by: Laine Taffin Altman <[email protected]>
> >> Signed-off-by: Benno Lossin <[email protected]>
> >
> > I think either in the commit log or in the code comment, there better be
> > a link or explanation on "(un)inhabited type". The rest looks good to
> > me.
>
> Would the following be okay for that purpose?
>
> A type is inhabited if at least one valid value of that type exists; a
> type is uninhabited if no valid values of that type exist. The terms
> "inhabited" and "uninhabited" in this sense originate in type theory,
> a branch of mathematics.
>
> In Rust, producing an invalid value of any type is immediate undefined
> behavior (UB); this includes via zeroing memory. Therefore, since an
> uninhabited type has no valid values, producing any values at all for
> it is UB.
>
> The Rust standard library type `core::convert::Infallible` is
> uninhabited, by virtue of having been declared as an enum with no
> cases, which always produces uninhabited types in Rust. Thus, remove
> the implementation of `Zeroable` for `Infallible`, thereby avoiding
> the UB.
>

Yeah, this works for me. Thanks!

Regards,
Boqun

> Thanks,
> Laine
>
> >
> > Reviewed-by: Boqun Feng <[email protected]>
> >
> > Regards,
> > Boqun
> >
> >> ---
> >> rust/kernel/init.rs | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
> >> index 424257284d16..538e03cfc84a 100644
> >> --- a/rust/kernel/init.rs
> >> +++ b/rust/kernel/init.rs
> >> @@ -1292,8 +1292,8 @@ macro_rules! impl_zeroable {
> >> i8, i16, i32, i64, i128, isize,
> >> f32, f64,
> >>
> >> - // SAFETY: These are ZSTs, there is nothing to zero.
> >> - {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, Infallible, (),
> >> + // SAFETY: These are inhabited ZSTs, there is nothing to zero and a valid value exists.
> >> + {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, (),
> >>
> >> // SAFETY: Type is allowed to take any value, including all zeros.
> >> {<T>} MaybeUninit<T>,
> >>
> >> base-commit: 768409cff6cc89fe1194da880537a09857b6e4db
> >> --
> >> 2.42.0
> >>
> >>
> >>
>

2024-03-19 05:29:02

by Laine Taffin Altman

[permalink] [raw]
Subject: Re: [PATCH] rust: init: remove impl Zeroable for Infallible

On Mar 18, 2024, at 9:39 PM, Boqun Feng <[email protected]> wrote:
> On Mon, Mar 18, 2024 at 08:17:07PM -0700, Laine Taffin Altman wrote:
>> On Mar 18, 2024, at 10:25 AM, Boqun Feng <[email protected]> wrote:
>>> On Wed, Mar 13, 2024 at 11:09:37PM +0000, Benno Lossin wrote:
>>>> From: Laine Taffin Altman <[email protected]>
>>>>
>>>> It is not enough for a type to be a ZST to guarantee that zeroed memory
>>>> is a valid value for it; it must also be inhabited. Creating a value of
>>>> an uninhabited type, ZST or no, is immediate UB.
>>>> Thus remove the implementation of `Zeroable` for `Infallible`, since
>>>> that type is not inhabited.
>>>>
>>>> Cc: [email protected]
>>>> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
>>>> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
>>>> Signed-off-by: Laine Taffin Altman <[email protected]>
>>>> Signed-off-by: Benno Lossin <[email protected]>
>>>
>>> I think either in the commit log or in the code comment, there better be
>>> a link or explanation on "(un)inhabited type". The rest looks good to
>>> me.
>>
>> Would the following be okay for that purpose?
>>
>> A type is inhabited if at least one valid value of that type exists; a
>> type is uninhabited if no valid values of that type exist. The terms
>> "inhabited" and "uninhabited" in this sense originate in type theory,
>> a branch of mathematics.
>>
>> In Rust, producing an invalid value of any type is immediate undefined
>> behavior (UB); this includes via zeroing memory. Therefore, since an
>> uninhabited type has no valid values, producing any values at all for
>> it is UB.
>>
>> The Rust standard library type `core::convert::Infallible` is
>> uninhabited, by virtue of having been declared as an enum with no
>> cases, which always produces uninhabited types in Rust. Thus, remove
>> the implementation of `Zeroable` for `Infallible`, thereby avoiding
>> the UB.
>>
>
> Yeah, this works for me. Thanks!

Great! Should it be re-sent or can the new wording be incorporated upon merge?

Thank,
Laine

>
> Regards,
> Boqun
>
>> Thanks,
>> Laine
>>
>>>
>>> Reviewed-by: Boqun Feng <[email protected]>
>>>
>>> Regards,
>>> Boqun
>>>
>>>> ---
>>>> rust/kernel/init.rs | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/rust/kernel/init.rs b/rust/kernel/init.rs
>>>> index 424257284d16..538e03cfc84a 100644
>>>> --- a/rust/kernel/init.rs
>>>> +++ b/rust/kernel/init.rs
>>>> @@ -1292,8 +1292,8 @@ macro_rules! impl_zeroable {
>>>> i8, i16, i32, i64, i128, isize,
>>>> f32, f64,
>>>>
>>>> - // SAFETY: These are ZSTs, there is nothing to zero.
>>>> - {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, Infallible, (),
>>>> + // SAFETY: These are inhabited ZSTs, there is nothing to zero and a valid value exists.
>>>> + {<T: ?Sized>} PhantomData<T>, core::marker::PhantomPinned, (),
>>>>
>>>> // SAFETY: Type is allowed to take any value, including all zeros.
>>>> {<T>} MaybeUninit<T>,
>>>>
>>>> base-commit: 768409cff6cc89fe1194da880537a09857b6e4db
>>>> --
>>>> 2.42.0
>>>>
>>>>
>>>>
>>


2024-03-19 10:34:47

by Benno Lossin

[permalink] [raw]
Subject: Re: [PATCH] rust: init: remove impl Zeroable for Infallible

On 3/19/24 06:28, Laine Taffin Altman wrote:
> On Mar 18, 2024, at 9:39 PM, Boqun Feng <[email protected]> wrote:
>> On Mon, Mar 18, 2024 at 08:17:07PM -0700, Laine Taffin Altman wrote:
>>> On Mar 18, 2024, at 10:25 AM, Boqun Feng <[email protected]> wrote:
>>>> On Wed, Mar 13, 2024 at 11:09:37PM +0000, Benno Lossin wrote:
>>>>> From: Laine Taffin Altman <[email protected]>
>>>>>
>>>>> It is not enough for a type to be a ZST to guarantee that zeroed memory
>>>>> is a valid value for it; it must also be inhabited. Creating a value of
>>>>> an uninhabited type, ZST or no, is immediate UB.
>>>>> Thus remove the implementation of `Zeroable` for `Infallible`, since
>>>>> that type is not inhabited.
>>>>>
>>>>> Cc: [email protected]
>>>>> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
>>>>> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
>>>>> Signed-off-by: Laine Taffin Altman <[email protected]>
>>>>> Signed-off-by: Benno Lossin <[email protected]>
>>>>
>>>> I think either in the commit log or in the code comment, there better be
>>>> a link or explanation on "(un)inhabited type". The rest looks good to
>>>> me.
>>>
>>> Would the following be okay for that purpose?
>>>
>>> A type is inhabited if at least one valid value of that type exists; a
>>> type is uninhabited if no valid values of that type exist. The terms
>>> "inhabited" and "uninhabited" in this sense originate in type theory,
>>> a branch of mathematics.
>>>
>>> In Rust, producing an invalid value of any type is immediate undefined
>>> behavior (UB); this includes via zeroing memory. Therefore, since an
>>> uninhabited type has no valid values, producing any values at all for
>>> it is UB.
>>>
>>> The Rust standard library type `core::convert::Infallible` is
>>> uninhabited, by virtue of having been declared as an enum with no
>>> cases, which always produces uninhabited types in Rust. Thus, remove
>>> the implementation of `Zeroable` for `Infallible`, thereby avoiding
>>> the UB.
>>>
>>
>> Yeah, this works for me. Thanks!
>
> Great! Should it be re-sent or can the new wording be incorporated upon merge?

I can re-send it for you again, or do you want to send it yourself?
I think it is also a good idea to add a link to [1] in the code, since
the above explanation is rather long and fits better in the commit
message.

--
Cheers,
Benno

[1]: https://doc.rust-lang.org/nomicon/exotic-sizes.html#empty-types


2024-03-19 11:24:53

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] rust: init: remove impl Zeroable for Infallible

On Tue, Mar 19, 2024 at 11:34 AM Benno Lossin <[email protected]> wrote:
>
> I can re-send it for you again, or do you want to send it yourself?
> I think it is also a good idea to add a link to [1] in the code, since
> the above explanation is rather long and fits better in the commit
> message.

Agreed, if you want to have a note in the code itself (to avoid
mistakes re-adding them, I imagine), then I would say a short sentence
+ link is best.

Your link is a good one for an explanation, since it mentions
explicitly the UB. The reference's list [1] would be a good fit for
non-explanation purposes -- it mentions explicitly `!` (and
`Infallible` is supposed to eventually be an alias as far as I know).

In addition, while it is not important in this case (i.e. most likely
nobody is affected), it doesn't hurt to include an example that shows
the issue in general for this sort of patches, i.e. what kind of code
will be prevented from compiling, e.g.

pr_info!("{}",
Box::<core::convert::Infallible>::init(kernel::init::zeroed())?);

In any case, even v1 looks good to me -- thanks!

[1] https://doc.rust-lang.org/reference/behavior-considered-undefined.html

Cheers,
Miguel

2024-03-21 04:54:09

by Laine Taffin Altman

[permalink] [raw]
Subject: Re: [PATCH] rust: init: remove impl Zeroable for Infallible

On Mar 19, 2024, at 3:34 AM, Benno Lossin <[email protected]> wrote:
> On 3/19/24 06:28, Laine Taffin Altman wrote:
>> On Mar 18, 2024, at 9:39 PM, Boqun Feng <[email protected]> wrote:
>>> On Mon, Mar 18, 2024 at 08:17:07PM -0700, Laine Taffin Altman wrote:
>>>> On Mar 18, 2024, at 10:25 AM, Boqun Feng <[email protected]> wrote:
>>>>> On Wed, Mar 13, 2024 at 11:09:37PM +0000, Benno Lossin wrote:
>>>>>> From: Laine Taffin Altman <[email protected]>
>>>>>>
>>>>>> It is not enough for a type to be a ZST to guarantee that zeroed memory
>>>>>> is a valid value for it; it must also be inhabited. Creating a value of
>>>>>> an uninhabited type, ZST or no, is immediate UB.
>>>>>> Thus remove the implementation of `Zeroable` for `Infallible`, since
>>>>>> that type is not inhabited.
>>>>>>
>>>>>> Cc: [email protected]
>>>>>> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
>>>>>> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
>>>>>> Signed-off-by: Laine Taffin Altman <[email protected]>
>>>>>> Signed-off-by: Benno Lossin <[email protected]>
>>>>>
>>>>> I think either in the commit log or in the code comment, there better be
>>>>> a link or explanation on "(un)inhabited type". The rest looks good to
>>>>> me.
>>>>
>>>> Would the following be okay for that purpose?
>>>>
>>>> A type is inhabited if at least one valid value of that type exists; a
>>>> type is uninhabited if no valid values of that type exist. The terms
>>>> "inhabited" and "uninhabited" in this sense originate in type theory,
>>>> a branch of mathematics.
>>>>
>>>> In Rust, producing an invalid value of any type is immediate undefined
>>>> behavior (UB); this includes via zeroing memory. Therefore, since an
>>>> uninhabited type has no valid values, producing any values at all for
>>>> it is UB.
>>>>
>>>> The Rust standard library type `core::convert::Infallible` is
>>>> uninhabited, by virtue of having been declared as an enum with no
>>>> cases, which always produces uninhabited types in Rust. Thus, remove
>>>> the implementation of `Zeroable` for `Infallible`, thereby avoiding
>>>> the UB.
>>>>
>>>
>>> Yeah, this works for me. Thanks!
>>
>> Great! Should it be re-sent or can the new wording be incorporated upon merge?
>
> I can re-send it for you again, or do you want to send it yourself?
> I think it is also a good idea to add a link to [1] in the code, since
> the above explanation is rather long and fits better in the commit
> message.
>

I’ll try and do it myself; thank you for sending the first round for me and illustrating procedures! What Reviewed-By’s/Signed-Off-By's should I retain?

Thanks,
Laine

> --
> Cheers,
> Benno
>
> [1]: https://doc.rust-lang.org/nomicon/exotic-sizes.html#empty-types



2024-03-21 09:13:49

by Miguel Ojeda

[permalink] [raw]
Subject: Re: [PATCH] rust: init: remove impl Zeroable for Infallible

On Thu, Mar 21, 2024 at 5:53 AM Laine Taffin Altman
<[email protected]> wrote:
>
> I’ll try and do it myself; thank you for sending the first round for me and illustrating procedures! What Reviewed-By’s/Signed-Off-By's should I retain?

For the Signed-off-by, only yours is OK. For the Reviewed-by, it
depends on how much you have changed, i.e. whether you consider their
review does not apply anymore -- please see
https://docs.kernel.org/process/submitting-patches.html#reviewer-s-statement-of-oversight

Cheers,
Miguel

2024-03-30 12:03:52

by Benno Lossin

[permalink] [raw]
Subject: Re: [PATCH] rust: init: remove impl Zeroable for Infallible

On 21.03.24 05:53, Laine Taffin Altman wrote:
> On Mar 19, 2024, at 3:34 AM, Benno Lossin <[email protected]> wrote:
>> On 3/19/24 06:28, Laine Taffin Altman wrote:
>>> On Mar 18, 2024, at 9:39 PM, Boqun Feng <[email protected]> wrote:
>>>> On Mon, Mar 18, 2024 at 08:17:07PM -0700, Laine Taffin Altman wrote:
>>>>> On Mar 18, 2024, at 10:25 AM, Boqun Feng <[email protected]> wrote:
>>>>>> On Wed, Mar 13, 2024 at 11:09:37PM +0000, Benno Lossin wrote:
>>>>>>> From: Laine Taffin Altman <[email protected]>
>>>>>>>
>>>>>>> It is not enough for a type to be a ZST to guarantee that zeroed memory
>>>>>>> is a valid value for it; it must also be inhabited. Creating a value of
>>>>>>> an uninhabited type, ZST or no, is immediate UB.
>>>>>>> Thus remove the implementation of `Zeroable` for `Infallible`, since
>>>>>>> that type is not inhabited.
>>>>>>>
>>>>>>> Cc: [email protected]
>>>>>>> Fixes: 38cde0bd7b67 ("rust: init: add `Zeroable` trait and `init::zeroed` function")
>>>>>>> Closes: https://github.com/Rust-for-Linux/pinned-init/pull/13
>>>>>>> Signed-off-by: Laine Taffin Altman <[email protected]>
>>>>>>> Signed-off-by: Benno Lossin <[email protected]>
>>>>>>
>>>>>> I think either in the commit log or in the code comment, there better be
>>>>>> a link or explanation on "(un)inhabited type". The rest looks good to
>>>>>> me.
>>>>>
>>>>> Would the following be okay for that purpose?
>>>>>
>>>>> A type is inhabited if at least one valid value of that type exists; a
>>>>> type is uninhabited if no valid values of that type exist. The terms
>>>>> "inhabited" and "uninhabited" in this sense originate in type theory,
>>>>> a branch of mathematics.
>>>>>
>>>>> In Rust, producing an invalid value of any type is immediate undefined
>>>>> behavior (UB); this includes via zeroing memory. Therefore, since an
>>>>> uninhabited type has no valid values, producing any values at all for
>>>>> it is UB.
>>>>>
>>>>> The Rust standard library type `core::convert::Infallible` is
>>>>> uninhabited, by virtue of having been declared as an enum with no
>>>>> cases, which always produces uninhabited types in Rust. Thus, remove
>>>>> the implementation of `Zeroable` for `Infallible`, thereby avoiding
>>>>> the UB.
>>>>>
>>>>
>>>> Yeah, this works for me. Thanks!
>>>
>>> Great! Should it be re-sent or can the new wording be incorporated upon merge?
>>
>> I can re-send it for you again, or do you want to send it yourself?
>> I think it is also a good idea to add a link to [1] in the code, since
>> the above explanation is rather long and fits better in the commit
>> message.
>>
>
> I’ll try and do it myself; thank you for sending the first round for me and illustrating procedures! What Reviewed-By’s/Signed-Off-By's should I retain?

Do you still want to send it yourself? If you don't have the time, no
problem, I can send it again.

--
Cheers,
Benno


2024-03-30 16:37:09

by Laine Taffin Altman

[permalink] [raw]
Subject: Re: [PATCH] rust: init: remove impl Zeroable for Infallible

I’ll do it myself tomorrow night; sorry for the delay!

Thanks,
Laine

On Mar 30, 2024, at 5:03 AM, Benno Lossin <[email protected]> wrote:
>
> 
> <mime-attachment>
> <encrypted.asc>

2024-03-30 16:43:49

by Benno Lossin

[permalink] [raw]
Subject: Re: [PATCH] rust: init: remove impl Zeroable for Infallible

On 30.03.24 17:36, Laine Taffin Altman wrote:
> I’ll do it myself tomorrow night; sorry for the delay!

Great, and no worries (it's fine if you still need a couple days)!

>
> Thanks,
> Laine
>
> On Mar 30, 2024, at 5:03 AM, Benno Lossin <[email protected]> wrote:
>>
>> 
>> <mime-attachment>
>> <encrypted.asc>

Sorry about that, I hope this one is not encrypted.

--
Cheers,
Benno