Add methods to go from a nul-terminated Vec<u8> to a CString #73139
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… and unchecked. Doc tests have been written and the documentation on the error type updated too.
|
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
tesuji
reviewed
Jun 9, 2020
poliorcetics
force-pushed
the
cstring-from-vec-with-nul
branch
from
7417ed5
to
7f3bb39
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
poliorcetics
force-pushed
the
cstring-from-vec-with-nul
branch
from
e9eddac
to
7f3bb39
Compare
|
I have a link error in CI that I do not have locally and I think it's because of an unstable flag that I misplaced/incorrectly used. #[unstable(feature = "cstring_from_vec_with_nul", issue = "73179")]
impl TryFrom<Vec<u8>> for CString {
type Error = FromBytesWithNulError;
/// See the document about [`from_vec_with_nul`] for more
/// informations about the behaviour of this method.
///
/// [`from_vec_with_nul`]: struct.CString.html#method.from_vec_with_nul
fn try_from(value: Vec<u8>) -> Result<Self, Self::Error> {
Self::from_vec_with_nul(value)
}
}The link impl CString {
#[unstable(feature = "cstring_from_vec_with_nul", issue = "73179")]
pub fn from_vec_with_nul(v: Vec<u8>) -> Result<Self, FromBytesWithNulError> { /* ... */ }
}I do not have any idea about how to fix that and was advised to ask @GuillaumeGomez. |
|
Replace |
|
@rustbot modify labels: A-ffi, C-feature-request, T-libs |
rustbot
added
A-FFI
dtolnay
requested changes
Jun 14, 2020
|
r? @dtolnay |
jyn514
reviewed
Jun 14, 2020
dtolnay
added
S-waiting-on-author
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
poliorcetics
force-pushed
the
cstring-from-vec-with-nul
branch
from
a426470
to
5f4eb27
Compare
|
On the recommendation of @dtolnay I added a new error type This type, defined as: #[derive(Clone, PartialEq, Eq, Debug)]
#[unstable(feature = "cstring_from_vec_with_nul", issue = "73179")]
pub struct FromVecWithNulError {
error_kind: FromBytesWithNulErrorKind,
bytes: Vec<u8>,
}It is inspired from the The #[unstable(feature = "cstring_from_vec_with_nul", issue = "73179")]
impl fmt::Display for FromVecWithNulError {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self.error_kind {
FromBytesWithNulErrorKind::InteriorNul(pos) => {
write!(f, "data provided contains an interior nul byte at pos {}", pos)
}
FromBytesWithNulErrorKind::NotNulTerminated => {
write!(f, "data provided is not nul terminated")
}
}
}
} |
|
@bors r+ |
|
📌 Commit 47cc5cc has been approved by |
bors
added
S-waiting-on-bors
|
I suggest squashing all the commits. |
|
@bors rollup |
RalfJung
added a commit
to RalfJung/rust
that referenced
this pull request
Jun 15, 2020
…nul, r=dtolnay Add methods to go from a nul-terminated Vec<u8> to a CString Fixes rust-lang#73100. Doc tests have been written and the documentation on the error type updated too. I used `#[stable(feature = "cstring_from_vec_with_nul", since = "1.46.0")]` but I don't know if the version is correct.
This was referenced Jun 15, 2020
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 15, 2020
Rollup of 10 pull requests Successful merges: - rust-lang#72707 (Use min_specialization in the remaining rustc crates) - rust-lang#72740 (On recursive ADT, provide indirection structured suggestion) - rust-lang#72879 (Miri: avoid tracking current location three times) - rust-lang#72938 (Stabilize Option::zip) - rust-lang#73086 (Rename "cyclone" to "apple-a7" per changes in upstream LLVM) - rust-lang#73104 (Example about explicit mutex dropping) - rust-lang#73139 (Add methods to go from a nul-terminated Vec<u8> to a CString) - rust-lang#73296 (Remove vestigial CI job msvc-aux.) - rust-lang#73304 (Revert heterogeneous SocketAddr PartialEq impls) - rust-lang#73331 (extend network support for HermitCore) Failed merges: r? @ghost
|
Should I still squash if it has been approved ? (Edit: and is being merged) |
|
Now it has been rolled up, so if that rollup succeeds the PR will be merged as-is. If the rollup fails, I'll r- so you can squash. |
|
Thanks, I'll know for the next time. :) |
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this pull request
Mar 19, 2022
…k-Simulacrum
add `CStr` method that accepts any slice containing a nul-terminated string
I haven't created an issue (tracking or otherwise) for this yet; apologies if my approach isn't correct. This is my first code contribution.
This change adds a member fn that converts a slice into a `CStr`; it is intended to be safer than `from_ptr` (which is unsafe and may read out of bounds), and more useful than `from_bytes_with_nul` (which requires that the caller already know where the nul byte is).
The reason I find this useful is for situations like this:
```rust
let mut buffer = [0u8; 32];
unsafe {
some_c_function(buffer.as_mut_ptr(), buffer.len());
}
let result = CStr::from_bytes_with_nul(&buffer).unwrap();
```
This code above returns an error with `kind = InteriorNul`, because `from_bytes_with_nul` expects that the caller has passed in a slice with the NUL byte at the end of the slice. But if I just got back a nul-terminated string from some FFI function, I probably don't know where the NUL byte is.
I would wish for a `CStr` constructor with the following properties:
- Accept `&[u8]` as input
- Scan for the first NUL byte and return the `CStr` that spans the correct sub-slice (see [future note below](rust-lang#94984 (comment))).
- Return an error if no NUL byte is found within the input slice
I asked on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/CStr.20from.20.26.5Bu8.5D.20without.20knowing.20the.20NUL.20location.3F) whether this sounded like a good idea, and got a couple of positive-sounding responses from `@joshtriplett` and `@AzureMarker.`
This is my first draft, so feedback is welcome.
A few issues that definitely need feedback:
1. Naming. `@joshtriplett` called this `from_bytes_with_internal_nul` on Zulip, but after staring at all of the available methods, I believe that this function is probably what end users want (rather than the existing fn `from_bytes_with_nul`). Giving it a simpler name (**`from_bytes`**) implies that this should be their first choice.
2. Should I add a similar method on `CString` that accepts `Vec<u8>`? I'd assume the answer is probably yes, but I figured I'd try to get early feedback before making this change bigger.
3. What should the error type look like? I made a unit struct since `CStr::from_bytes` can only fail in one obvious way, but if I need to do this for `CString` as well then that one may want to return `FromVecWithNulError`. And maybe that should dictate the shape of the `CStr` error type also?
Also, cc `@poliorcetics` who wrote rust-lang#73139 containing similar fns.
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this pull request
Mar 19, 2022
…k-Simulacrum
add `CStr` method that accepts any slice containing a nul-terminated string
I haven't created an issue (tracking or otherwise) for this yet; apologies if my approach isn't correct. This is my first code contribution.
This change adds a member fn that converts a slice into a `CStr`; it is intended to be safer than `from_ptr` (which is unsafe and may read out of bounds), and more useful than `from_bytes_with_nul` (which requires that the caller already know where the nul byte is).
The reason I find this useful is for situations like this:
```rust
let mut buffer = [0u8; 32];
unsafe {
some_c_function(buffer.as_mut_ptr(), buffer.len());
}
let result = CStr::from_bytes_with_nul(&buffer).unwrap();
```
This code above returns an error with `kind = InteriorNul`, because `from_bytes_with_nul` expects that the caller has passed in a slice with the NUL byte at the end of the slice. But if I just got back a nul-terminated string from some FFI function, I probably don't know where the NUL byte is.
I would wish for a `CStr` constructor with the following properties:
- Accept `&[u8]` as input
- Scan for the first NUL byte and return the `CStr` that spans the correct sub-slice (see [future note below](rust-lang#94984 (comment))).
- Return an error if no NUL byte is found within the input slice
I asked on [Zulip](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/CStr.20from.20.26.5Bu8.5D.20without.20knowing.20the.20NUL.20location.3F) whether this sounded like a good idea, and got a couple of positive-sounding responses from ``@joshtriplett`` and ``@AzureMarker.``
This is my first draft, so feedback is welcome.
A few issues that definitely need feedback:
1. Naming. ``@joshtriplett`` called this `from_bytes_with_internal_nul` on Zulip, but after staring at all of the available methods, I believe that this function is probably what end users want (rather than the existing fn `from_bytes_with_nul`). Giving it a simpler name (**`from_bytes`**) implies that this should be their first choice.
2. Should I add a similar method on `CString` that accepts `Vec<u8>`? I'd assume the answer is probably yes, but I figured I'd try to get early feedback before making this change bigger.
3. What should the error type look like? I made a unit struct since `CStr::from_bytes` can only fail in one obvious way, but if I need to do this for `CString` as well then that one may want to return `FromVecWithNulError`. And maybe that should dictate the shape of the `CStr` error type also?
Also, cc ``@poliorcetics`` who wrote rust-lang#73139 containing similar fns.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #73100.
Doc tests have been written and the documentation on the error type
updated too.
I used
#[stable(feature = "cstring_from_vec_with_nul", since = "1.46.0")]but I don't know if the version is correct.