Skip to content

rust: types: Add try_from_foreign() method #1059

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

Closed
wants to merge 1 commit into from

Conversation

obeis
Copy link

@obeis obeis commented Jan 22, 2024

Close #1057

@obeis obeis force-pushed the try-from-foreign branch 2 times, most recently from 5ff8680 to 534b8e8 Compare January 22, 2024 08:55
@ojeda
Copy link
Member

ojeda commented Jan 22, 2024

Thanks!

The commit should be signed (in the Signed-off-by sense, please see https://docs.kernel.org/process/submitting-patches.html#developer-s-certificate-of-origin-1-1) and should have a Link: tag to the issue.

The commit message could perhaps also show how a user would look like (like one of the ones linked in the issue).

Apart from that, it seems OK to submit it to the mailing list -- please see https://rust-for-linux.com/contributing.

obeis added a commit to obeis/linux that referenced this pull request Jan 22, 2024
Currently `ForeignOwnable::from_foreign()` only
works for non-null pointers for the existing
impls (e.g. Box, Arc). It may create a few
duplicate code like:

```rust
// `p` is a maybe null pointer
if p.is_null() {
  None
} else {
  Some(unsafe { T::from_foreign(p) })
}
``

Link: Rust-for-Linux#1059

Cc: Alice Ryhl <[email protected]>
Cc: Matt Gilbride <[email protected]>
Cc: Miguel Ojeda <[email protected]>
Signed-off-by: Obei Sideg <[email protected]>
obeis added a commit to obeis/linux that referenced this pull request Jan 22, 2024
Currently `ForeignOwnable::from_foreign()` only
works for non-null pointers for the existing
impls (e.g. Box, Arc). It may create a few
duplicate code like:

```rust
// `p` is a maybe null pointer
if p.is_null() {
  None
} else {
  Some(unsafe { T::from_foreign(p) })
}
``

Link: Rust-for-Linux#1059

Cc: Alice Ryhl <[email protected]>
Cc: Matt Gilbride <[email protected]>
Cc: Miguel Ojeda <[email protected]>
Signed-off-by: Obei Sideg <[email protected]>
@charmitro
Copy link

Looks good. Is there code in the tree that can use this right away or we'd have to wait for patches to get merged?

@obeis obeis force-pushed the try-from-foreign branch 6 times, most recently from ef4a066 to 63eb0b3 Compare January 29, 2024 06:42
Currently `ForeignOwnable::from_foreign()` only works for non-null
pointers for the existing impls (e.g. Box, Arc). It may create a few
duplicate code like:

```rust
// `p` is a maybe null pointer
if p.is_null() {
  None
} else {
  unsafe { Some(Self::from_foreign(ptr)) }
}
```

Add a `try_from_foreign()` method that will return `None` if `ptr`
is null, otherwise return `Some(from_foreign(ptr))`.

Link: Rust-for-Linux#1057
Signed-off-by: Obei Sideg <[email protected]>
Reviewed-by: Alice Ryhl <[email protected]>
Reviewed-by: Trevor Gross <[email protected]>
@ojeda
Copy link
Member

ojeda commented Feb 26, 2024

try_from_foreign (option 1 in the linked issue) is now in rust-next.

@obeis
Copy link
Author

obeis commented Apr 16, 2024

@obeis obeis closed this Apr 16, 2024
@obeis obeis deleted the try-from-foreign branch April 16, 2024 11:27
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.

try_from_foreign()
3 participants