error checking tweaks

(cherry picked from commit 6ac38519b710e69a0c30eb0fe8fc5fa712168cb8)

rust/nix-expr/src/eval_state.rs

@@ -52,8 +52,9 @@ impl EvalState {
                 store.raw_ptr(),
             )
         };
+        context.check_err()?;
         if eval_state.is_null() {
-            bail!("nix_state_create returned a null pointer");
+            panic!("nix_state_create returned a null pointer without an error");
         }
         Ok(EvalState {
             eval_state: NonNull::new(eval_state).unwrap(),
@@ -152,7 +153,7 @@ where
     if unsafe { raw::GC_thread_is_registered() } != 0 {
         return Ok(f());
     } else {
-        gc_register_my_thread().unwrap();
+        gc_register_my_thread()?;
         let r = f();
         unsafe {
             raw::GC_unregister_my_thread();

rust/nix-expr/src/value.rs

@@ -1,6 +1,6 @@
 use nix_c_raw as raw;
 use nix_util::context::Context;
-use std::ptr::NonNull;
+use std::ptr::{null_mut, NonNull};
 
 // TODO: test: cloning a thunk does not duplicate the evaluation.
 
@@ -35,6 +35,7 @@ impl ValueType {
             raw::ValueType_NIX_TYPE_PATH => ValueType::Path,
             raw::ValueType_NIX_TYPE_STRING => ValueType::String,
             raw::ValueType_NIX_TYPE_THUNK => ValueType::Thunk,
+            // This would happen if a new type of value is added in Nix.
             _ => ValueType::Unknown,
         }
     }
@@ -56,17 +57,17 @@ impl Value {
 }
 impl Drop for Value {
     fn drop(&mut self) {
-        let context = Context::new();
         unsafe {
-            raw::gc_decref(context.ptr(), self.inner.as_ptr());
+            // ignoring error because the only failure mode is leaking memory
+            raw::gc_decref(null_mut(), self.inner.as_ptr());
         }
-        // ignore error from context, because drop should not panic
     }
 }
 impl Clone for Value {
     fn clone(&self) -> Self {
         let context = Context::new();
         unsafe { raw::gc_incref(context.ptr(), self.inner.as_ptr()) };
+        // can't return an error here, but we don't want to ignore the error either as it means we could use-after-free
         context.check_err().unwrap();
         Value { inner: self.inner }
     }

rust/nix-store/src/store.rs

@@ -62,7 +62,7 @@ impl Store {
         };
         context.check_err()?;
         if store.is_null() {
-            bail!("nix_c_store_open returned a null pointer");
+            panic!("nix_c_store_open returned a null pointer without an error");
         }
         let store = Store {
             inner: StoreRef {

rust/nix-util/src/context.rs

@@ -11,6 +11,8 @@ impl Context {
     pub fn new() -> Self {
         let ctx = unsafe { raw::c_context_create() };
         if ctx.is_null() {
+            // We've failed to allocate a (relatively small) Context struct.
+            // We're almost certainly going to crash anyways.
             panic!("nix_c_context_create returned a null pointer");
         }
         let ctx = Context {
@@ -24,7 +26,7 @@ impl Context {
     pub fn check_err(&self) -> Result<()> {
         let err = unsafe { raw::err_code(self.inner.as_ptr()) };
         if err != raw::NIX_OK.try_into().unwrap() {
-            // msgp is a borrowed pointer, so we don't need to free it
+            // msgp is a borrowed pointer (pointing into the context), so we don't need to free it
             let msgp = unsafe { raw::err_msg(null_mut(), self.inner.as_ptr(), null_mut()) };
             // Turn the i8 pointer into a Rust string by copying
             let msg: &str = unsafe { core::ffi::CStr::from_ptr(msgp).to_str()? };

rust/nix-util/src/string_return.rs

@@ -1,6 +1,8 @@
 /// Callback for nix_store_get_uri and other functions that return a string.
 ///
 /// This function is used by the other nix_* crates, and you should never need to call it yourself.
+///
+/// Some functions in the nix library "return" strings without giving you ownership over them, by letting you pass a callback function that gets to look at that string. This callback simply turns that string pointer into an owned rust String.
 pub unsafe extern "C" fn callback_get_vec_u8(
     start: *const ::std::os::raw::c_char,
     n: std::os::raw::c_uint,