Skip to content

Commit aacf02f

Browse files
committed
Auto merge of #134143 - nyurik:err-nul, r=<try>
Convert `struct FromBytesWithNulError` into enum This PR renames the former `kind` enum from `FromBytesWithNulErrorKind` to `FromBytesWithNulError`, and removes the original struct. See rust-lang/libs-team#493 ## Possible Changes - TBD * [x] should the new `enum FromBytesWithNulError` derive `Copy`? * [ ] should there be any new/changed attributes? * [x] add some more tests ## Problem One of `CStr` constructors, `CStr::from_bytes_with_nul(bytes: &[u8])` handles 3 cases: 1. `bytes` has one NULL as the last value - creates CStr 2. `bytes` has no NULL - error 3. `bytes` has a NULL in some other position - error The 3rd case is error that may require lossy conversion, but the 2nd case can easily be handled by the user code. Unfortunately, this function returns an opaque `FromBytesWithNulError` error in both 2nd and 3rd case, so the user cannot detect just the 2nd case - having to re-implement the entire function and bring in the `memchr` dependency. ## Motivating examples or use cases In [this code](https://github.com/gquintard/varnish-rs/blob/f86d7a87683b08d2e634d63e77d9dc1d24ed4a13/varnish-sys/src/vcl/ws.rs#L158), my FFI code needs to copy user's `&[u8]` into a C-allocated memory blob in a NUL-terminated `CStr` format. My code must first validate if `&[u8]` has a trailing NUL (case 1), no NUL (adds one on the fly - case 2), or NUL in the middle (3rd case - error). I had to re-implement `from_bytes_with_nul` and add `memchr`dependency just to handle the 2nd case. r? `@Amanieu`
2 parents 8417f83 + bf4ecec commit aacf02f

File tree

1 file changed

+19
-31
lines changed

1 file changed

+19
-31
lines changed

Diff for: library/core/src/ffi/c_str.rs

+19-31
Original file line numberDiff line numberDiff line change
@@ -124,37 +124,25 @@ pub struct CStr {
124124
///
125125
/// let _: FromBytesWithNulError = CStr::from_bytes_with_nul(b"f\0oo").unwrap_err();
126126
/// ```
127-
#[derive(Clone, PartialEq, Eq, Debug)]
127+
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
128128
#[stable(feature = "core_c_str", since = "1.64.0")]
129-
pub struct FromBytesWithNulError {
130-
kind: FromBytesWithNulErrorKind,
131-
}
132-
133-
#[derive(Clone, PartialEq, Eq, Debug)]
134-
enum FromBytesWithNulErrorKind {
135-
InteriorNul(usize),
129+
pub enum FromBytesWithNulError {
130+
/// Data provided contains an interior nul byte at byte `pos`.
131+
InteriorNul {
132+
/// The position of the interior nul byte.
133+
pos: usize,
134+
},
135+
/// Data provided is not nul terminated.
136136
NotNulTerminated,
137137
}
138138

139-
// FIXME: const stability attributes should not be required here, I think
140-
impl FromBytesWithNulError {
141-
const fn interior_nul(pos: usize) -> FromBytesWithNulError {
142-
FromBytesWithNulError { kind: FromBytesWithNulErrorKind::InteriorNul(pos) }
143-
}
144-
const fn not_nul_terminated() -> FromBytesWithNulError {
145-
FromBytesWithNulError { kind: FromBytesWithNulErrorKind::NotNulTerminated }
146-
}
147-
}
148-
149139
#[stable(feature = "frombyteswithnulerror_impls", since = "1.17.0")]
150140
impl Error for FromBytesWithNulError {
151141
#[allow(deprecated)]
152142
fn description(&self) -> &str {
153-
match self.kind {
154-
FromBytesWithNulErrorKind::InteriorNul(..) => {
155-
"data provided contains an interior nul byte"
156-
}
157-
FromBytesWithNulErrorKind::NotNulTerminated => "data provided is not nul terminated",
143+
match self {
144+
Self::InteriorNul { .. } => "data provided contains an interior nul byte",
145+
Self::NotNulTerminated => "data provided is not nul terminated",
158146
}
159147
}
160148
}
@@ -199,7 +187,7 @@ impl fmt::Display for FromBytesWithNulError {
199187
#[allow(deprecated, deprecated_in_future)]
200188
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
201189
f.write_str(self.description())?;
202-
if let FromBytesWithNulErrorKind::InteriorNul(pos) = self.kind {
190+
if let Self::InteriorNul { pos } = self {
203191
write!(f, " at byte pos {pos}")?;
204192
}
205193
Ok(())
@@ -349,25 +337,25 @@ impl CStr {
349337
/// use std::ffi::CStr;
350338
///
351339
/// let cstr = CStr::from_bytes_with_nul(b"hello\0");
352-
/// assert!(cstr.is_ok());
340+
/// assert_eq!(cstr, Ok(c"hello"));
353341
/// ```
354342
///
355343
/// Creating a `CStr` without a trailing nul terminator is an error:
356344
///
357345
/// ```
358-
/// use std::ffi::CStr;
346+
/// use std::ffi::{CStr, FromBytesWithNulError};
359347
///
360348
/// let cstr = CStr::from_bytes_with_nul(b"hello");
361-
/// assert!(cstr.is_err());
349+
/// assert_eq!(cstr, Err(FromBytesWithNulError::NotNulTerminated));
362350
/// ```
363351
///
364352
/// Creating a `CStr` with an interior nul byte is an error:
365353
///
366354
/// ```
367-
/// use std::ffi::CStr;
355+
/// use std::ffi::{CStr, FromBytesWithNulError};
368356
///
369357
/// let cstr = CStr::from_bytes_with_nul(b"he\0llo\0");
370-
/// assert!(cstr.is_err());
358+
/// assert_eq!(cstr, Err(FromBytesWithNulError::InteriorNul { pos: 2 }));
371359
/// ```
372360
#[stable(feature = "cstr_from_bytes", since = "1.10.0")]
373361
#[rustc_const_stable(feature = "const_cstr_methods", since = "1.72.0")]
@@ -379,8 +367,8 @@ impl CStr {
379367
// of the byte slice.
380368
Ok(unsafe { Self::from_bytes_with_nul_unchecked(bytes) })
381369
}
382-
Some(nul_pos) => Err(FromBytesWithNulError::interior_nul(nul_pos)),
383-
None => Err(FromBytesWithNulError::not_nul_terminated()),
370+
Some(pos) => Err(FromBytesWithNulError::InteriorNul { pos }),
371+
None => Err(FromBytesWithNulError::NotNulTerminated),
384372
}
385373
}
386374

0 commit comments

Comments
 (0)