From 15c4b87202b8e4a0f0e3ccc96dad99217ab519e7 Mon Sep 17 00:00:00 2001
From: Yuri Astrakhan <YuriAstrakhan@gmail.com>
Date: Tue, 10 Dec 2024 15:45:23 -0500
Subject: [PATCH] Convert `struct FromBytesWithNulError` into enum

## 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.
---
 library/core/src/ffi/c_str.rs | 33 +++++++++------------------------
 1 file changed, 9 insertions(+), 24 deletions(-)

diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs
index 8831443a10f17..b6e27423f0569 100644
--- a/library/core/src/ffi/c_str.rs
+++ b/library/core/src/ffi/c_str.rs
@@ -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",
         }
     }
 }
@@ -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(())
@@ -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),
         }
     }