refactor: moved OsStr->CString->ptr utilities to a separate module

Author: joe-saronic

src/conv.rs

@@ -8,11 +8,13 @@
 //! convrnx(StreamFmt::Rinex, &mut opt, "input.rnx", &ofiles).unwrap();
 //! ```
 
-use crate::{GpsTime, NavSys};
+use crate::{
+    util::{copy_osstr, CStringArray},
+    GpsTime, NavSys,
+};
 use num_enum::TryFromPrimitive;
 use rtklib_sys::rtklib as ffi;
-use std::ffi::{CString, OsStr, OsString};
-use std::os::unix::ffi::OsStrExt;
+use std::ffi::{OsStr, OsString};
 use thiserror::Error;
 
 /// Input stream format for [`convrnx`].
@@ -133,15 +135,6 @@ pub enum RinexVersion {
     V305 = 305,
 }
 
-fn copy_osstr<const N: usize>(dst: &mut [i8; N], src: &OsStr) {
-    let src = src.as_bytes();
-    let n = src.len().min(N - 1);
-    unsafe {
-        std::ptr::copy_nonoverlapping(src.as_ptr() as *const i8, dst.as_mut_ptr(), n);
-        dst[n] = 0;
-    }
-}
-
 /// RINEX conversion options.
 #[derive(Clone, Copy)]
 pub struct RnxOpt(ffi::rnxopt_t);
@@ -446,23 +439,15 @@ pub fn convrnx(
     file: impl AsRef<OsStr>,
     ofile: &RnxOutputFiles,
 ) -> Result<(), ConvrnxError> {
-    let file = file.as_ref();
-    let file_c =
-        CString::new(file.as_bytes()).map_err(|_| ConvrnxError::NulByte(file.to_owned()))?;
-
-    let mut cstrings = ofile
-        .as_slice()
-        .iter()
-        .map(|p| CString::new(p.as_bytes()).map_err(|_| ConvrnxError::NulByte(p.clone())))
-        .collect::<Result<Vec<_>, _>>()?;
-    let mut ptrs: Vec<*mut i8> = cstrings.iter().map(|s| s.as_ptr() as *mut i8).collect();
+    let file_arr = CStringArray::try_single(file.as_ref()).map_err(|e| ConvrnxError::NulByte(e.to_owned()))?;
+    let mut ofile_arr = CStringArray::try_new(ofile.as_slice()).map_err(|e| ConvrnxError::NulByte(e.to_owned()))?;
 
     let ret = unsafe {
         ffi::convrnx(
             format as i32,
             &mut opt.0,
-            file_c.as_ptr(),
-            ptrs.as_mut_ptr(),
+            file_arr.first(),
+            ofile_arr.as_mut_ptr() as *mut *mut i8,
         )
     };
 

src/lib.rs

@@ -37,6 +37,8 @@ pub use solution::*;
 pub mod meas;
 pub use meas::*;
 
+mod util;
+
 /// Error returned when a decoder fails to initialize.
 #[derive(Debug, thiserror::Error)]
 #[error("failed to initialize decoder")]

src/ppk.rs

@@ -14,7 +14,7 @@
 //! ).unwrap();
 //! ```
 
-use crate::NavSys;
+use crate::{util::CStringArray, NavSys};
 use num_enum::TryFromPrimitive;
 use rtklib_sys::rtklib as ffi;
 use std::ffi::CString;
@@ -427,26 +427,22 @@ pub fn postpos(
         });
     }
 
-    let mut cstrings = Vec::with_capacity(total);
-    let mut paths = vec![rover_obs, base_obs];
-    paths.extend_from_slice(nav_files);
-
-    for p in &paths {
-        cstrings.push(CString::new(*p).map_err(|_| PostposError::NulByte(p.to_string()))?);
-    }
+    let mut all_inputs = vec![rover_obs, base_obs];
+    all_inputs.extend_from_slice(nav_files);
 
     // C signature is `const char **infile` — the strings are const but the
     // pointer to the array is not, so bindgen generates `*mut *const c_char`.
     // The function does not actually mutate the array.
-    let mut ptrs: Vec<*const i8> = cstrings.iter().map(|s| s.as_ptr()).collect();
-    let outfile = CString::new(output).map_err(|_| PostposError::NulByte(output.to_string()))?;
+    let mut infile_arr = CStringArray::try_new(&all_inputs)
+        .map_err(|e| PostposError::NulByte(e.to_string_lossy().into_owned()))?;
+    let out_arr = CStringArray::try_new(&[output])
+        .map_err(|e| PostposError::NulByte(e.to_string_lossy().into_owned()))?;
+    let rov = CString::new("").unwrap();
+    let base = CString::new("").unwrap();
 
     let ts = ffi::gtime_t { time: 0, sec: 0.0 };
     let te = ffi::gtime_t { time: 0, sec: 0.0 };
 
-    let rov = CString::new("").unwrap();
-    let base = CString::new("").unwrap();
-
     let ret = unsafe {
         ffi::postpos(
             ts,
@@ -456,9 +452,9 @@ pub fn postpos(
             popt.as_ffi(),
             sopt.as_ffi(),
             fopt.as_ffi(),
-            ptrs.as_mut_ptr(),
-            ptrs.len() as i32,
-            outfile.as_ptr(),
+            infile_arr.as_mut_ptr(),
+            infile_arr.len() as i32,
+            out_arr.first(),
             rov.as_ptr(),
             base.as_ptr(),
         )

src/solution.rs

@@ -4,11 +4,10 @@
 //! [`read_solt`] wraps `readsolt`, and [`write_sol`] wraps `outsolheads` and
 //! `outsols` from `solution.c`.
 
-use crate::{ppk::SolOpt, GpsTime, Llh, SolStatus};
+use crate::{ppk::SolOpt, util::CStringArray, GpsTime, Llh, SolStatus};
 use num_enum::TryFromPrimitive;
 use rtklib_sys::rtklib as ffi;
 use std::{
-    ffi::CString,
     fs::File,
     io::{Error as IoError, Write},
 };
@@ -195,20 +194,12 @@ fn validate_coord_types(buf: SolBuf) -> Result<SolBuf, SolError> {
     Ok(buf)
 }
 
-fn build_cstring_ptrs(paths: &[&str]) -> Result<(Vec<CString>, Vec<*const i8>), SolError> {
-    let cstrings = paths
-        .iter()
-        .map(|p| CString::new(*p).map_err(|_| SolError::NulByte(p.to_string())))
-        .collect::<Result<Vec<_>, _>>()?;
-    let ptrs = cstrings.iter().map(|s| s.as_ptr()).collect();
-    Ok((cstrings, ptrs))
-}
-
 /// Read solution records from one or more `.pos` files.
 pub fn read_sol(paths: &[&str]) -> Result<SolBuf, SolError> {
-    let (_cstrings, mut ptrs) = build_cstring_ptrs(paths)?;
+    let mut arr = CStringArray::try_new(paths)
+        .map_err(|e| SolError::NulByte(e.to_string_lossy().into_owned()))?;
     let mut buf = unsafe { std::mem::zeroed::<ffi::solbuf_t>() };
-    let ret = unsafe { ffi::readsol(ptrs.as_mut_ptr(), ptrs.len() as i32, &mut buf) };
+    let ret = unsafe { ffi::readsol(arr.as_mut_ptr(), arr.len() as i32, &mut buf) };
     if ret == 0 {
         return Err(SolError::ReadFailed);
     }
@@ -224,12 +215,13 @@ pub fn read_solt(
     qflag: i32,
     mean: bool,
 ) -> Result<SolBuf, SolError> {
-    let (_cstrings, mut ptrs) = build_cstring_ptrs(paths)?;
+    let mut arr = CStringArray::try_new(paths)
+        .map_err(|e| SolError::NulByte(e.to_string_lossy().into_owned()))?;
     let mut buf = unsafe { std::mem::zeroed::<ffi::solbuf_t>() };
     let ret = unsafe {
         ffi::readsolt(
-            ptrs.as_mut_ptr(),
-            ptrs.len() as i32,
+            arr.as_mut_ptr(),
+            arr.len() as i32,
             ts.0,
             te.0,
             tint,

src/util.rs

@@ -0,0 +1,63 @@
+use std::{
+    ffi::{CString, OsStr},
+    os::unix::ffi::OsStrExt,
+};
+
+/// Copy an `OsStr` into a fixed-size null-terminated `[i8; N]` C buffer.
+/// Truncates silently if `src` is longer than `N - 1` bytes.
+pub(crate) fn copy_osstr<const N: usize>(dst: &mut [i8; N], src: &OsStr) {
+    let src = src.as_bytes();
+    let n = src.len().min(N - 1);
+    unsafe {
+        std::ptr::copy_nonoverlapping(src.as_ptr() as *const i8, dst.as_mut_ptr(), n);
+        dst[n] = 0;
+    }
+}
+
+/// Owned array of C strings with a stable pointer list for FFI calls.
+///
+/// `as_ptr` holds pointers into each `CString`'s owned string buffer. Moving this
+/// struct retains the validity of the pointers to the heap-allocated strings.
+pub(crate) struct CStringArray {
+    _strings: Vec<CString>,
+    ptrs: Vec<*const i8>,
+}
+
+impl CStringArray {
+    /// Build from a slice of paths. Returns the offending path if one contains a NUL byte.
+    pub(crate) fn try_new<T: AsRef<OsStr>>(paths: &[T]) -> Result<Self, &OsStr> {
+        let strings = paths
+            .iter()
+            .map(|p| {
+                let os = p.as_ref();
+                CString::new(os.as_bytes()).map_err(|_| os)
+            })
+            .collect::<Result<Vec<_>, _>>()?;
+        let ptrs = strings.iter().map(|s| s.as_ptr()).collect();
+        Ok(Self { _strings: strings, ptrs })
+    }
+
+    /// Build from a single path. Returns the offending path if it contains a NUL byte.
+    pub(crate) fn try_single(path: &OsStr) -> Result<Self, &OsStr> {
+        let string = CString::new(path.as_bytes()).map_err(|_| path)?;
+        Ok(Self {
+            ptrs: vec![string.as_ptr()],
+            _strings: vec![string],
+        })
+    }
+
+    /// Pointer to the first string. Panics if empty.
+    pub(crate) fn first(&self) -> *const i8 {
+        self.ptrs[0]
+    }
+
+    /// Mutable pointer to the start of the pointer array, for passing to C as `char **`.
+    pub(crate) fn as_mut_ptr(&mut self) -> *mut *const i8 {
+        self.ptrs.as_mut_ptr()
+    }
+
+    /// Number of strings in the array.
+    pub(crate) fn len(&self) -> usize {
+        self.ptrs.len()
+    }
+}