Skip to content

Commit

Permalink
Convert struct FromBytesWithNulError into enum
Browse files Browse the repository at this point in the history
## Description

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.

## Solution

This PR renames the former `kind` enum from `FromBytesWithNulErrorKind` to `FromBytesWithNulError`, and removes the original struct.
  • Loading branch information
nyurik committed Dec 25, 2024
1 parent 7c002ff commit 92f7a12
Showing 1 changed file with 14 additions and 29 deletions.
43 changes: 14 additions & 29 deletions library/core/src/ffi/c_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,35 +126,20 @@ pub struct CStr {
/// ```
#[derive(Clone, PartialEq, Eq, Debug)]
#[stable(feature = "core_c_str", since = "1.64.0")]
pub struct FromBytesWithNulError {
kind: FromBytesWithNulErrorKind,
}

#[derive(Clone, PartialEq, Eq, Debug)]
enum FromBytesWithNulErrorKind {
pub enum FromBytesWithNulError {
/// Data provided contains an interior nul byte at byte pos `usize`.
InteriorNul(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",
}
}
}
Expand Down Expand Up @@ -199,7 +184,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(())
Expand Down Expand Up @@ -349,25 +334,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(2)));
/// ```
#[stable(feature = "cstr_from_bytes", since = "1.10.0")]
#[rustc_const_stable(feature = "const_cstr_methods", since = "1.72.0")]
Expand All @@ -379,8 +364,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(nul_pos) => Err(FromBytesWithNulError::InteriorNul(nul_pos)),
None => Err(FromBytesWithNulError::NotNulTerminated),
}
}

Expand Down

0 comments on commit 92f7a12

Please sign in to comment.