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

Implement Deref<Target=ByteStr> for CStr #138498

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tamird
Copy link
Contributor

@tamird tamird commented Mar 14, 2025

This produces a deref chain of CStr -> BStr -> [u8] which is
present in the Rust-for-Linux analogues of these types.

Link: #134915
Link: Rust-for-Linux/linux#1075
Link: https://lore.kernel.org/all/20250221142816.0c015e9f@eugeo/
Link: Rust-for-Linux/linux#1146

r? @joshtriplett
cc @nbdd0121

I'm not entirely sure what it means for a stable type to deref to an unstable one - hopefully the compiler does the right thing and gates this impl on all the items being available.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 14, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

This produces a deref chain of `CStr` -> `BStr` -> `[u8]` which is
present in the Rust-for-Linux analogues of these types.

Link: rust-lang#134915
Link: Rust-for-Linux/linux#1075
Link: https://lore.kernel.org/all/20250221142816.0c015e9f@eugeo/
Link: Rust-for-Linux/linux#1146
@joshtriplett
Copy link
Member

This is a really interesting idea. One potential point of concern: the deref to ByteStr uses to_bytes (rather than to_bytes_with_nul), which somewhat makes sense, but with the further deref to &[u8], is there any chance that someone will end up with a &[u8] that lacks the NUL when they expected a &[u8] that has the NUL?

This is probably fine, and I would hope that any FFI cases that require the NUL use CStr (or to_bytes_with_nul). However, I wanted to call it out as a potential hazard. If we do this, I think this deserves a mention in the documentation at least.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants