Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

alloc: restrict impl ZeroableOption for Box to T: Sized #32

Merged
merged 2 commits into from
Apr 1, 2025

Conversation

ojeda
Copy link
Member

@ojeda ojeda commented Mar 31, 2025

Similar to what was done for Zeroable<NonNull<T>> in commit 9caa350 ("last docs changes and zeroable unsized pointer fixes"), the latest Rust documentation [1] says it guarantees that transmute::<_, Option<T>>([0u8; size_of::<T>()]) is sound and produces Option::<T>::None only in some cases. In particular, it says:

`Box<U>` (specifically, only `Box<U, Global>`) when `U: Sized`

Thus restrict the impl to Sized, and use similar wording as in that commit too.

Link: https://doc.rust-lang.org/stable/std/option/index.html#representation [1]

@ojeda
Copy link
Member Author

ojeda commented Mar 31, 2025

For the kernel side, if you decide to send it as a fix, then the fix tag would be:

Fixes: 9b2299af3b92 ("rust: pin-init: add `std` and `alloc` support from the user-space version")

@ojeda ojeda force-pushed the zeroable-option-box-fix branch from 9314dca to b41aad0 Compare March 31, 2025 21:06
Similar to what was done for `Zeroable<NonNull<T>>` in commit 9caa350
("last docs changes and zeroable unsized pointer fixes"), the latest Rust
documentation [1] says it guarantees that `transmute::<_, Option<T>>([0u8;
size_of::<T>()])` is sound and produces `Option::<T>::None` only in some
cases. In particular, it says:

    `Box<U>` (specifically, only `Box<U, Global>`) when `U: Sized`

Thus restrict the `impl` to `Sized`, and use similar wording as in that
commit too.

Link: https://doc.rust-lang.org/stable/std/option/index.html#representation [1]
Signed-off-by: Miguel Ojeda <[email protected]>
@ojeda ojeda force-pushed the zeroable-option-box-fix branch from b41aad0 to a6007cf Compare April 1, 2025 16:34
It allows, for instance, kitty to pick it up as links properly [1].

Suggested-by: Benno Lossin <[email protected]>
Link: Rust-for-Linux#32 (comment) [1]
Signed-off-by: Miguel Ojeda <[email protected]>
Copy link
Member

@y86-dev y86-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fixes!

@y86-dev y86-dev merged commit fdc7061 into Rust-for-Linux:main Apr 1, 2025
26 checks passed
@ojeda ojeda deleted the zeroable-option-box-fix branch April 1, 2025 22:19
y86-dev pushed a commit to y86-dev/linux that referenced this pull request Apr 2, 2025
It allows, for instance, kitty to pick it up as links properly [1].

Suggested-by: Benno Lossin <[email protected]>
Link: Rust-for-Linux/pin-init#32 (comment) [1]
Signed-off-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux/pin-init@dd230d6
[ Change case in title. - Benno ]
Signed-off-by: Benno Lossin <[email protected]>
y86-dev pushed a commit to y86-dev/linux that referenced this pull request Apr 2, 2025
"Normal" comments in Rust (`//`) are also formatted in Markdown, like
the documentation (`///` and `//!`), see
Documentation/rust/coding-guidelines.rst

Thus use Markdown autolinks for a couple links that were missing it.

It also helps to get proper linking in some software like kitty [1].

Suggested-by: Benno Lossin <[email protected]>
Link: Rust-for-Linux/pin-init#32 (comment) [1]
Signed-off-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux/pin-init@dd230d6
[ Change case in title. Reworded commit message. - Benno ]
Signed-off-by: Benno Lossin <[email protected]>
y86-dev pushed a commit to y86-dev/linux that referenced this pull request Apr 7, 2025
"Normal" comments in Rust (`//`) are also formatted in Markdown, like
the documentation (`///` and `//!`), see
Documentation/rust/coding-guidelines.rst

Thus use Markdown autolinks for a couple links that were missing it.

It also helps to get proper linking in some software like kitty [1].

Suggested-by: Benno Lossin <[email protected]>
Link: Rust-for-Linux/pin-init#32 (comment) [1]
Signed-off-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux/pin-init@dd230d6
Fixes: 84837cf ("rust: pin-init: change examples to the user-space version")
[ Change case in title. Reworded commit message. - Benno ]
Signed-off-by: Benno Lossin <[email protected]>
y86-dev pushed a commit to y86-dev/linux that referenced this pull request Apr 7, 2025
"Normal" comments in Rust (`//`) are also formatted in Markdown, like
the documentation (`///` and `//!`), see
Documentation/rust/coding-guidelines.rst

Thus use Markdown autolinks for a couple links that were missing it.

It also helps to get proper linking in some software like kitty [1].

Suggested-by: Benno Lossin <[email protected]>
Link: Rust-for-Linux/pin-init#32 (comment) [1]
Signed-off-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux/pin-init@dd230d6
Fixes: 84837cf ("rust: pin-init: change examples to the user-space version")
Cc: [email protected]
[ Change case in title. Reworded commit message. - Benno ]
Signed-off-by: Benno Lossin <[email protected]>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this pull request Apr 7, 2025
"Normal" comments in Rust (`//`) are also formatted in Markdown, like
the documentation (`///` and `//!`), see
Documentation/rust/coding-guidelines.rst

Thus use Markdown autolinks for a couple links that were missing it.

It also helps to get proper linking in some software like kitty [1].

Suggested-by: Benno Lossin <[email protected]>
Link: Rust-for-Linux/pin-init#32 (comment) [1]
Signed-off-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux/pin-init@dd230d6
Fixes: 84837cf ("rust: pin-init: change examples to the user-space version")
Cc: [email protected]
[ Change case in title. Reworded commit message. - Benno ]
Signed-off-by: Benno Lossin <[email protected]>
ojeda added a commit to ojeda/linux that referenced this pull request Apr 8, 2025
"Normal" comments in Rust (`//`) are also formatted in Markdown, like
the documentation (`///` and `//!`), see
Documentation/rust/coding-guidelines.rst

Thus use Markdown autolinks for a couple links that were missing it.

It also helps to get proper linking in some software like kitty [1].

Suggested-by: Benno Lossin <[email protected]>
Link: Rust-for-Linux/pin-init#32 (comment) [1]
Signed-off-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux/pin-init@dd230d6
Fixes: 84837cf ("rust: pin-init: change examples to the user-space version")
Cc: [email protected]
[ Change case in title. Reworded commit message. - Benno ]
Signed-off-by: Benno Lossin <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Miguel Ojeda <[email protected]>
ojeda added a commit to Rust-for-Linux/linux that referenced this pull request Apr 8, 2025
"Normal" comments in Rust (`//`) are also formatted in Markdown, like
the documentation (`///` and `//!`), see
Documentation/rust/coding-guidelines.rst

Thus use Markdown autolinks for a couple links that were missing it.

It also helps to get proper linking in some software like kitty [1].

Suggested-by: Benno Lossin <[email protected]>
Link: Rust-for-Linux/pin-init#32 (comment) [1]
Signed-off-by: Miguel Ojeda <[email protected]>
Link: Rust-for-Linux/pin-init@dd230d6
Fixes: 84837cf ("rust: pin-init: change examples to the user-space version")
Cc: [email protected]
[ Change case in title. Reworded commit message. - Benno ]
Signed-off-by: Benno Lossin <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Miguel Ojeda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants