Skip to content

Replace CStr::from_ptr() with CStr::from_bytes_with_nul()#82

Merged
Nercury merged 1 commit into
masterfrom
cstr
Mar 4, 2025
Merged

Replace CStr::from_ptr() with CStr::from_bytes_with_nul()#82
Nercury merged 1 commit into
masterfrom
cstr

Conversation

@MarijnS95

Copy link
Copy Markdown
Member

For #81 (comment)

CStr::from_ptr() is unsafe because it reads a raw pointer, and searches for a terminating nul character in the pointed region of memory.

This is unnecessary as both calls already initialize a given number of characters and terminate with a nul, allowing us to pass a sized and initialized slice (without casting *const MaybeUninit<u8> to *const u8) directly to CStr::from_bytes_with_nul() (available since Rust 1.10, unlike CStr::from_bytes_until_nul() which was only stabilized in 1.69). Unfortunately all std helper APIs to initialize slices of MaybeUninit are still unstable, making this less ideal to write at the moment.

Comment thread src/lib.rs Outdated
Comment thread src/lib.rs Outdated
@Nercury

Nercury commented Mar 4, 2025

Copy link
Copy Markdown
Collaborator

Sorry for noticing your PR a bit late. Just ping me next time, it's ok. Let me know when you are ready for review.

@MarijnS95

MarijnS95 commented Mar 4, 2025

Copy link
Copy Markdown
Member Author

@Nercury nice timing; I was updating it as we speak after noticing I left this sit idle :). It's ready for review now.

@Nercury

Nercury commented Mar 4, 2025

Copy link
Copy Markdown
Collaborator

Whoopsie it has merge conflict now.

`CStr::from_ptr()` is `unsafe` because it reads a raw pointer, and
searches for a terminating nul character in the pointed region of
memory.

This is unnecessary as both calls already initialize a given number of
characters and terminate with a nul, allowing us to pass a sized and
initialized slice (without casting `*const MaybeUninit<u8>` to `*const
u8`) directly to `CStr::from_bytes_with_nul()` (available since Rust
1.10, unlike `CStr::from_bytes_until_nul()` which was only stabilized
in 1.69).  Unfortunately all `std` helper APIs to initialize slices of
`MaybeUninit` are still unstable, making this less ideal to write at
the moment.
@MarijnS95 MarijnS95 requested a review from Nercury March 4, 2025 14:16
@Nercury Nercury merged commit 401a08d into master Mar 4, 2025
@MarijnS95 MarijnS95 deleted the cstr branch March 4, 2025 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants