diff --git a/nixide/src/expr/eval_state_builder.rs b/nixide/src/expr/eval_state_builder.rs index 6933de7..117cf8a 100644 --- a/nixide/src/expr/eval_state_builder.rs +++ b/nixide/src/expr/eval_state_builder.rs @@ -1,7 +1,5 @@ use std::cell::RefCell; -use std::ffi::CString; -use std::hint; -use std::iter; +use std::ffi::{CString, c_char}; use std::ptr::{self, NonNull}; use std::rc::Rc; @@ -9,10 +7,10 @@ use super::EvalState; #[cfg(feature = "flakes")] use crate::FlakeSettings; use crate::Store; -use crate::errors::{ErrorContext, NixideResult, ToNixideResult as _}; +use crate::errors::{ErrorContext, NixideResult}; use crate::sys; -use crate::util::wrap; use crate::util::wrappers::AsInnerPtr; +use crate::util::{panic_issue_call_failed, wrap}; /// Builder for Nix evaluation state. /// @@ -106,38 +104,47 @@ impl EvalStateBuilder { } pub fn set_lookup_path>(self, paths: Vec

) -> NixideResult { - // WARNING: Creating a c-style array requires leaking each path - // WARNING: then appending a null terminator. Otherwise Rust - // WARNING: will free the paths and leave dangling pointers. - let (c_paths, paths_len, paths_cap) = paths + // let paths_len = paths.len(); + // let paths_capacity = paths.capacity(); + + // XXX: TODO: use the `AsCArray` trait instead + let mut ptrs: Vec<*const c_char> = paths .into_iter() - .map(|p| -> NixideResult<_> { + .map(|p| { CString::new(p.as_ref()) - .to_nixide_result() - .map(|s| s.into_raw().cast_const()) + .unwrap_or_else(|err| { + panic_issue_call_failed!( + "given string {} contains a NUL byte ({})", + p.as_ref(), + err + ) + }) + .into_raw() as *const c_char }) - .chain(iter::once(Ok(ptr::null()))) - .collect::>>()? - .into_raw_parts(); + .collect(); - let builder = wrap::nix_fn!(|ctx: &ErrorContext| unsafe { - sys::nix_eval_state_builder_set_lookup_path(ctx.as_ptr(), self.as_ptr(), c_paths); - self - })?; + ptrs.push(ptr::null()); - // WARNING: We must reconstruct the leaked elements manually. - // WARNING: Using the `black_box` compiler hint ensures this - // WARNING: block is never compiled away. - unsafe { - hint::black_box( - Vec::from_raw_parts(c_paths, paths_len, paths_cap) - .into_iter() - .map(|p| hint::black_box(CString::from_raw(p.cast_mut()))) - .collect::>(), - ); - } + // Leak the Vec and return as mutable pointer + let ptr = ptrs.as_mut_ptr(); + std::mem::forget(ptrs); - Ok(builder) + let result = wrap::nix_fn!(|ctx: &ErrorContext| unsafe { + sys::nix_eval_state_builder_set_lookup_path(ctx.as_ptr(), self.as_ptr(), ptr); + }) + .map(|()| self); + + // ensure all allocated memory is dropped + // XXX: TODO!! + // unsafe { + // Vec::from_raw_parts(ptr, paths_len, paths_capacity) + // .into_iter() + // .map(|p| { + // _ = CString::from_raw(p as *mut c_char); + // }) + // }; + + result } } diff --git a/nixide/src/store/path.rs b/nixide/src/store/path.rs index c0dea21..b316997 100644 --- a/nixide/src/store/path.rs +++ b/nixide/src/store/path.rs @@ -6,7 +6,7 @@ use std::rc::Rc; use super::Store; use crate::NixideResult; -use crate::errors::{ErrorContext, ToNixideResult as _, new_nixide_error}; +use crate::errors::{ErrorContext, new_nixide_error}; use crate::stdext::CCharPtrExt as _; use crate::sys; use crate::util::panic_issue_call_failed; @@ -85,7 +85,7 @@ impl StorePath { /// ``` /// pub fn new(store: Rc>, path: &str) -> NixideResult { - let c_path = CString::new(path).to_nixide_result()?; + let c_path = CString::new(path).or(Err(new_nixide_error!(StringNulByte)))?; let inner = wrap::nix_ptr_fn!(|ctx: &ErrorContext| unsafe { sys::nix_store_parse_path(ctx.as_ptr(), store.borrow().as_ptr(), c_path.as_ptr())