Some CStr cleanup

* remove NULL-char related comments when using `CStr` - that's the definition of it.
* casting `cstr.as_ptr()` with `.cast::<c_char>()` is redundant - it's already that.
* modify `WS::copy_bytes_with_null` to return `CStr`, and marked it as `unsafe`. Current implementation relies on the input value to not contain any `\0` chars, and would result in UB otherwise.

Overall, I think current string handling needs some work - `VCL_STRING` seems to be equivalent to `CStr`, and Varnish guarantees(?) it not to have NULLs. If so, `CStr` could be treated as a tier-1 type together (allowed by callback functions as arg and return).

examples/vmod_vfp/src/lib.rs

@@ -32,7 +32,7 @@ impl VFP for Lower {
         pull_res
     }
 
-    // return our id, adding the NULL character to avoid confusing the C layer
+    // return our id
     fn name() -> &'static CStr {
         c"lower"
     }

src/vcl/backend.rs

@@ -467,7 +467,7 @@ unsafe extern "C" fn wrap_gethdrs<S: Serve<T>, T: Transfer>(
                             ));
                             return -1;
                         };
-                        (*vfp).name = t.as_ptr().cast::<c_char>();
+                        (*vfp).name = t.as_ptr();
                         (*vfp).init = None;
                         (*vfp).pull = Some(vfp_pull::<T>);
                         (*vfp).fini = None;

src/vcl/convert.rs

@@ -150,7 +150,7 @@ impl IntoVCL<VCL_STRING> for &[u8] {
         } {
             Ok(self.as_ptr().cast::<c_char>())
         } else {
-            Ok(ws.copy_bytes_with_null(&self)?.as_ptr().cast::<c_char>())
+            Ok(ws.copy_bytes_with_null(&self)?.as_ptr())
         }
     }
 }

src/vcl/http.rs

@@ -12,7 +12,7 @@
 //! this [issue](https://github.com/gquintard/varnish-rs/issues/4).
 
 #![allow(clippy::not_unsafe_ptr_arg_deref)]
-use std::ffi::{c_char, c_uint};
+use std::ffi::c_uint;
 use std::slice::{from_raw_parts, from_raw_parts_mut};
 use std::str::from_utf8;
 
@@ -48,12 +48,12 @@ impl<'a> HTTP<'a> {
 
         /* XXX: aliasing warning, it's the same pointer as the one in Ctx */
         let mut ws = WS::new(self.raw.ws);
-        let hdr_buf = ws.copy_bytes_with_null(&value)?;
         unsafe {
+            let hdr_buf = ws.copy_bytes_with_null(&value)?;
             let hd = self.raw.hd.offset(idx as isize);
-            (*hd).b = hdr_buf.as_ptr().cast::<c_char>();
-            /* -1 accounts for the null character */
-            (*hd).e = hdr_buf.as_ptr().add(hdr_buf.len() - 1).cast::<c_char>();
+            (*hd).b = hdr_buf.as_ptr();
+            // .e points to the NULL byte at the end of the string
+            (*hd).e = hdr_buf.as_ptr().add(hdr_buf.count_bytes());
             let hdf = self.raw.hdf.offset(idx as isize);
             *hdf = 0;
         }

src/vcl/processor.rs

@@ -5,7 +5,7 @@
 //! *Note:* The rust wrapper here is pretty thin and the vmod writer will most probably need to have to
 //! deal with the raw Varnish internals.
 
-use std::ffi::{c_char, c_int, c_uint, c_void, CStr};
+use std::ffi::{c_int, c_uint, c_void, CStr};
 use std::ptr;
 
 use crate::ffi;
@@ -66,8 +66,6 @@ where
     /// [`VDPCtx::push`] to push data to the next processor.
     fn push(&mut self, ctx: &mut VDPCtx, act: PushAction, buf: &[u8]) -> PushResult;
     /// The name of the processor.
-    ///
-    /// **Note:** it must be NULL-terminated as it will be used directly as a C string.
     fn name() -> &'static CStr;
 }
 
@@ -136,7 +134,7 @@ pub unsafe extern "C" fn gen_vdp_push<T: VDP>(
 /// Create a `ffi::vdp` that can be fed to `ffi::VRT_AddVDP`
 pub fn new_vdp<T: VDP>() -> ffi::vdp {
     ffi::vdp {
-        name: T::name().as_ptr().cast::<c_char>(),
+        name: T::name().as_ptr(),
         init: Some(gen_vdp_init::<T>),
         bytes: Some(gen_vdp_push::<T>),
         fini: Some(gen_vdp_fini::<T>),
@@ -191,8 +189,6 @@ where
     /// processor.
     fn pull(&mut self, ctx: &mut VFPCtx, buf: &mut [u8]) -> PullResult;
     /// The name of the processor.
-    ///
-    /// **Note:** it must be NULL-terminated as it will be used directly as a C string.
     fn name() -> &'static CStr;
 }
 
@@ -268,7 +264,7 @@ pub unsafe extern "C" fn wrap_vfp_fini<T: VFP>(ctxp: *mut vfp_ctx, vfep: *mut vf
 /// Create a `ffi::vfp` that can be fed to `ffi::VRT_AddVFP`
 pub fn new_vfp<T: VFP>() -> ffi::vfp {
     ffi::vfp {
-        name: T::name().as_ptr().cast::<c_char>(),
+        name: T::name().as_ptr(),
         init: Some(wrap_vfp_init::<T>),
         pull: Some(wrap_vfp_pull::<T>),
         fini: Some(wrap_vfp_fini::<T>),

src/vcl/ws.rs

@@ -10,7 +10,7 @@
 //! **Note:** unless you know what you are doing, you should probably just use the automatic type
 //! conversion provided by [`crate::vcl::convert`], or store things in
 //! [`crate::vcl::vpriv::VPriv`].
-use std::ffi::{c_char, c_void};
+use std::ffi::{c_char, c_void, CStr};
 use std::marker::PhantomData;
 use std::ptr;
 use std::slice::from_raw_parts_mut;
@@ -54,6 +54,7 @@ impl<'a> WS<'a> {
             unsafe { Ok(from_raw_parts_mut(p, sz)) }
         }
     }
+
     #[cfg(test)]
     pub fn alloc(&mut self, sz: usize) -> Result<&'a mut [u8], String> {
         let wsp = unsafe { self.raw.as_mut().unwrap() };
@@ -87,15 +88,18 @@ impl<'a> WS<'a> {
     }
 
     /// Same as [`WS::copy_bytes`] but adds NULL character at the end to help converts buffers into
-    /// `VCL_STRING`s
-    pub fn copy_bytes_with_null<T: AsRef<[u8]>>(&mut self, src: &T) -> Result<&'a [u8], String> {
+    /// `VCL_STRING`s. Returns an error if `src` contain NULL characters.
+    pub fn copy_bytes_with_null<T: AsRef<[u8]>>(&mut self, src: &T) -> Result<&'a CStr, String> {
         let buf = src.as_ref();
+        if buf.contains(&0) {
+            return Err("source buffer contains NULL character".into());
+        }
         let l = buf.len();
-
         let dest = self.alloc(l + 1)?;
         dest[..l].copy_from_slice(buf);
         dest[l] = b'\0';
-        Ok(dest)
+        // Safe because there are no NULLs in the source, and we just added a NULL
+        Ok(unsafe { CStr::from_bytes_with_nul_unchecked(dest) })
     }
 
     /// Copy any "`str`-like" struct into the workspace