From 2f5a3d4b06a0a9e8749c2e361758ec517df0c96a Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Tue, 10 Dec 2024 15:45:23 -0500 Subject: [PATCH 1/2] Convert `struct FromBytesWithNulError` into enum 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. 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. This PR renames the former `kind` enum from `FromBytesWithNulErrorKind` to `FromBytesWithNulError`, and removes the original struct. --- library/core/src/ffi/c_str.rs | 50 +++++++++++++---------------------- 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index 8831443a10f17..63915c35288a3 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -124,37 +124,25 @@ pub struct CStr { /// /// let _: FromBytesWithNulError = CStr::from_bytes_with_nul(b"f\0oo").unwrap_err(); /// ``` -#[derive(Clone, PartialEq, Eq, Debug)] +#[derive(Clone, Copy, PartialEq, Eq, Debug)] #[stable(feature = "core_c_str", since = "1.64.0")] -pub struct FromBytesWithNulError { - kind: FromBytesWithNulErrorKind, -} - -#[derive(Clone, PartialEq, Eq, Debug)] -enum FromBytesWithNulErrorKind { - InteriorNul(usize), +pub enum FromBytesWithNulError { + /// Data provided contains an interior nul byte at byte `pos`. + InteriorNul { + /// The position of the interior nul byte. + pos: usize, + }, + /// Data provided is not nul terminated. NotNulTerminated, } -// FIXME: const stability attributes should not be required here, I think -impl FromBytesWithNulError { - const fn interior_nul(pos: usize) -> FromBytesWithNulError { - FromBytesWithNulError { kind: FromBytesWithNulErrorKind::InteriorNul(pos) } - } - const fn not_nul_terminated() -> FromBytesWithNulError { - FromBytesWithNulError { kind: FromBytesWithNulErrorKind::NotNulTerminated } - } -} - #[stable(feature = "frombyteswithnulerror_impls", since = "1.17.0")] impl Error for FromBytesWithNulError { #[allow(deprecated)] fn description(&self) -> &str { - match self.kind { - FromBytesWithNulErrorKind::InteriorNul(..) => { - "data provided contains an interior nul byte" - } - FromBytesWithNulErrorKind::NotNulTerminated => "data provided is not nul terminated", + match self { + Self::InteriorNul { .. } => "data provided contains an interior nul byte", + Self::NotNulTerminated => "data provided is not nul terminated", } } } @@ -199,7 +187,7 @@ impl fmt::Display for FromBytesWithNulError { #[allow(deprecated, deprecated_in_future)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str(self.description())?; - if let FromBytesWithNulErrorKind::InteriorNul(pos) = self.kind { + if let Self::InteriorNul { pos } = self { write!(f, " at byte pos {pos}")?; } Ok(()) @@ -349,25 +337,25 @@ impl CStr { /// use std::ffi::CStr; /// /// let cstr = CStr::from_bytes_with_nul(b"hello\0"); - /// assert!(cstr.is_ok()); + /// assert_eq!(cstr, Ok(c"hello")); /// ``` /// /// Creating a `CStr` without a trailing nul terminator is an error: /// /// ``` - /// use std::ffi::CStr; + /// use std::ffi::{CStr, FromBytesWithNulError}; /// /// let cstr = CStr::from_bytes_with_nul(b"hello"); - /// assert!(cstr.is_err()); + /// assert_eq!(cstr, Err(FromBytesWithNulError::NotNulTerminated)); /// ``` /// /// Creating a `CStr` with an interior nul byte is an error: /// /// ``` - /// use std::ffi::CStr; + /// use std::ffi::{CStr, FromBytesWithNulError}; /// /// let cstr = CStr::from_bytes_with_nul(b"he\0llo\0"); - /// assert!(cstr.is_err()); + /// assert_eq!(cstr, Err(FromBytesWithNulError::InteriorNul { pos: 2 })); /// ``` #[stable(feature = "cstr_from_bytes", since = "1.10.0")] #[rustc_const_stable(feature = "const_cstr_methods", since = "1.72.0")] @@ -379,8 +367,8 @@ impl CStr { // of the byte slice. Ok(unsafe { Self::from_bytes_with_nul_unchecked(bytes) }) } - Some(nul_pos) => Err(FromBytesWithNulError::interior_nul(nul_pos)), - None => Err(FromBytesWithNulError::not_nul_terminated()), + Some(pos) => Err(FromBytesWithNulError::InteriorNul { pos }), + None => Err(FromBytesWithNulError::NotNulTerminated), } } From 86b86fa8fb89aca91aa176a0727b9f233cd14353 Mon Sep 17 00:00:00 2001 From: Yuri Astrakhan Date: Sat, 11 Jan 2025 02:56:58 -0500 Subject: [PATCH 2/2] Rename `pos` to `position` --- library/core/src/ffi/c_str.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index 63915c35288a3..7180593edf0d0 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -127,10 +127,10 @@ pub struct CStr { #[derive(Clone, Copy, PartialEq, Eq, Debug)] #[stable(feature = "core_c_str", since = "1.64.0")] pub enum FromBytesWithNulError { - /// Data provided contains an interior nul byte at byte `pos`. + /// Data provided contains an interior nul byte at byte `position`. InteriorNul { /// The position of the interior nul byte. - pos: usize, + position: usize, }, /// Data provided is not nul terminated. NotNulTerminated, @@ -187,8 +187,8 @@ impl fmt::Display for FromBytesWithNulError { #[allow(deprecated, deprecated_in_future)] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str(self.description())?; - if let Self::InteriorNul { pos } = self { - write!(f, " at byte pos {pos}")?; + if let Self::InteriorNul { position } = self { + write!(f, " at byte pos {position}")?; } Ok(()) } @@ -355,7 +355,7 @@ impl CStr { /// use std::ffi::{CStr, FromBytesWithNulError}; /// /// let cstr = CStr::from_bytes_with_nul(b"he\0llo\0"); - /// assert_eq!(cstr, Err(FromBytesWithNulError::InteriorNul { pos: 2 })); + /// assert_eq!(cstr, Err(FromBytesWithNulError::InteriorNul { position: 2 })); /// ``` #[stable(feature = "cstr_from_bytes", since = "1.10.0")] #[rustc_const_stable(feature = "const_cstr_methods", since = "1.72.0")] @@ -367,7 +367,7 @@ impl CStr { // of the byte slice. Ok(unsafe { Self::from_bytes_with_nul_unchecked(bytes) }) } - Some(pos) => Err(FromBytesWithNulError::InteriorNul { pos }), + Some(position) => Err(FromBytesWithNulError::InteriorNul { position }), None => Err(FromBytesWithNulError::NotNulTerminated), } }