refactor: Remove check_one_call

(cherry picked from commit 065f880e52c6d6cb44e4b857272176ebe2464eea)

rust/nix-expr/src/eval_state.rs

@@ -16,10 +16,9 @@ lazy_static! {
     static ref INIT: Result<()> = {
         unsafe {
             raw::GC_allow_register_threads();
+            check_call!(raw::libexpr_init[&mut Context::new()])?;
+            Ok(())
         }
-        Context::new().check_one_call(|ctx_ptr| unsafe {
-            raw::libexpr_init(ctx_ptr);
-        })
     };
 }
 pub fn init() -> Result<()> {
@@ -67,9 +66,9 @@ impl EvalState {
 
         init()?;
 
-        let eval_state = context.check_one_call(|ctx_ptr| unsafe {
+        let eval_state = unsafe {
-            raw::state_create(ctx_ptr, lookup_path.as_mut_ptr(), store.raw_ptr())
+            check_call!(raw::state_create[&mut context, lookup_path.as_mut_ptr(), store.raw_ptr()])
-        })?;
+        }?;
         Ok(EvalState {
             eval_state: NonNull::new(eval_state).unwrap_or_else(|| {
                 panic!("nix_state_create returned a null pointer without an error")
@@ -128,9 +127,12 @@ impl EvalState {
         Ok(())
     }
     pub fn value_type_unforced(&mut self, value: &Value) -> Option<ValueType> {
-        let r = self
+        let r = unsafe {
-            .context
+            check_call!(raw::get_type[
-            .check_one_call(|ctx_ptr| unsafe { raw::get_type(ctx_ptr, value.raw_ptr()) });
+                &mut self.context,
+                value.raw_ptr()
+            ])
+        };
         // .unwrap(): no reason for this to fail, as it does not evaluate
         ValueType::from_raw(r.unwrap())
     }
@@ -153,8 +155,7 @@ impl EvalState {
         if t != ValueType::Int {
             bail!("expected an int, but got a {:?}", t);
         }
-        self.context
+        unsafe { check_call!(raw::get_int[&mut self.context, v.raw_ptr()]) }
-            .check_one_call(|ctx_ptr| unsafe { raw::get_int(ctx_ptr, v.raw_ptr()) })
     }
     /// Evaluate, and require that the value is an attrset.
     /// Returns a list of the keys in the attrset.
@@ -163,10 +164,8 @@ impl EvalState {
         if t != ValueType::AttrSet {
             bail!("expected an attrset, but got a {:?}", t);
         }
-        let n = self.context.check_one_call(|ctx_ptr| unsafe {
+        let n = unsafe { check_call!(raw::get_attrs_size[&mut self.context, v.raw_ptr()]) }?;
-            raw::get_attrs_size(ctx_ptr, v.raw_ptr()) as usize
+        let mut attrs = Vec::with_capacity(n as usize);
-        })?;
-        let mut attrs = Vec::with_capacity(n);
         for i in 0..n {
             let cstr_ptr: *const i8 = unsafe {
                 check_call!(raw::get_attr_name_byidx[
@@ -180,7 +179,7 @@ impl EvalState {
             let s = cstr
                 .to_str()
                 .map_err(|e| anyhow::format_err!("Nix attrset key is not valid UTF-8: {}", e))?;
-            attrs.insert(i, s.to_owned());
+            attrs.insert(i as usize, s.to_owned());
         }
         Ok(attrs)
     }
@@ -239,8 +238,7 @@ impl EvalState {
         let s = CString::new(s).with_context(|| "new_value_str: contains null byte")?;
         let v = unsafe {
             let value = self.new_value_uninitialized()?;
-            self.context
+            check_call!(raw::init_string[&mut self.context, value.raw_ptr(), s.as_ptr()])?;
-                .check_one_call(|ctx_ptr| raw::init_string(ctx_ptr, value.raw_ptr(), s.as_ptr()))?;
             value
         };
         Ok(v)
@@ -249,8 +247,7 @@ impl EvalState {
     pub fn new_value_int(&mut self, i: Int) -> Result<Value> {
         let v = unsafe {
             let value = self.new_value_uninitialized()?;
-            self.context
+            check_call!(raw::init_int[&mut self.context, value.raw_ptr(), i])?;
-                .check_one_call(|ctx_ptr| raw::init_int(ctx_ptr, value.raw_ptr(), i))?;
             value
         };
         Ok(v)
@@ -260,14 +257,12 @@ impl EvalState {
     fn get_string(&mut self, value: &Value) -> Result<String> {
         let mut r = result_string_init!();
         unsafe {
-            self.context.check_one_call(|ctx_ptr| {
+            check_call!(raw::get_string[
-                raw::get_string(
+                &mut self.context,
-                    ctx_ptr,
+                value.raw_ptr(),
-                    value.raw_ptr(),
+                Some(callback_get_result_string),
-                    Some(callback_get_result_string),
+                callback_get_result_string_data(&mut r)
-                    callback_get_result_string_data(&mut r),
+            ])?;
-                )
-            })?;
         };
         r
     }

rust/nix-expr/src/value.rs

@@ -1,5 +1,5 @@
 use nix_c_raw as raw;
-use nix_util::context::Context;
+use nix_util::{check_call, context::Context};
 use std::ptr::{null_mut, NonNull};
 
 // TODO: test: cloning a thunk does not duplicate the evaluation.
@@ -73,10 +73,12 @@ impl Drop for Value {
 }
 impl Clone for Value {
     fn clone(&self) -> Self {
-        let mut context = Context::new();
+        // TODO: Is it worth allocating a new Context here? Ideally cloning is cheap.
-        context
+        //       this is very unlikely to error, and it is not recoverable
-            .check_one_call(|ctx_ptr| unsafe { raw::gc_incref(ctx_ptr, self.inner.as_ptr()) })
+        //       Maybe try without, and try again with context to report details?
-            .unwrap();
+        unsafe {
+            check_call!(raw::gc_incref[&mut Context::new(), self.inner.as_ptr()]).unwrap();
+        }
         // can't return an error here, but we don't want to ignore the error either as it means we could use-after-free
         Value { inner: self.inner }
     }

rust/nix-store/src/store.rs

@@ -2,18 +2,17 @@ use anyhow::{bail, Result};
 use lazy_static::lazy_static;
 use nix_c_raw as raw;
 use nix_util::context::Context;
-use nix_util::result_string_init;
 use nix_util::string_return::{callback_get_result_string, callback_get_result_string_data};
+use nix_util::{check_call, result_string_init};
 use std::ffi::{c_char, CString};
 use std::ptr::null_mut;
 use std::ptr::NonNull;
 
 /* TODO make Nix itself thread safe */
 lazy_static! {
-    static ref INIT: Result<()> = {
+    static ref INIT: Result<()> = unsafe {
-        Context::new().check_one_call(|ctx_ptr| unsafe {
+        check_call!(raw::libstore_init[&mut Context::new()])?;
-            raw::libstore_init(ctx_ptr);
+        Ok(())
-        })
     };
 }
 
@@ -74,9 +73,9 @@ impl Store {
             .chain(std::iter::once(null_mut())) // signal the end of the array
             .collect();
 
-        let store = context.check_one_call(|ctx_ptr| unsafe {
+        let store = unsafe {
-            raw::store_open(ctx_ptr, uri_ptr.as_ptr(), params.as_mut_ptr())
+            check_call!(raw::store_open[&mut context, uri_ptr.as_ptr(), params.as_mut_ptr()])
-        })?;
+        }?;
         if store.is_null() {
             panic!("nix_c_store_open returned a null pointer without an error");
         }
@@ -95,14 +94,9 @@ impl Store {
 
     pub fn get_uri(&mut self) -> Result<String> {
         let mut r = result_string_init!();
-        self.context.check_one_call(|ctx_ptr| unsafe {
+        unsafe {
-            raw::store_get_uri(
+            check_call!(raw::store_get_uri[&mut self.context, self.inner.ptr(), Some(callback_get_result_string), callback_get_result_string_data(&mut r)])
-                ctx_ptr,
+        }?;
-                self.inner.ptr(),
-                Some(callback_get_result_string),
-                callback_get_result_string_data(&mut r),
-            )
-        })?;
         r
     }
 }

rust/nix-util/src/context.rs

@@ -61,15 +61,6 @@ impl Context {
         r
     }
 
-    /// Run the function, and check the error, then reset the error.
-    /// Make at most one call to a Nix function in `f`.
-    /// Do not use if the context isn't fresh or cleared (e.g. with `check_err_and_clear`).
-    pub fn check_one_call<T, F: FnOnce(*mut raw::c_context) -> T>(&mut self, f: F) -> Result<T> {
-        let t = f(self.ptr());
-        self.check_err_and_clear()?;
-        Ok(t)
-    }
-
     pub fn check_one_call_or_key_none<T, F: FnOnce(*mut raw::c_context) -> T>(
         &mut self,
         f: F,

rust/nix-util/src/settings.rs

@@ -2,7 +2,7 @@ use anyhow::Result;
 use nix_c_raw as raw;
 
 use crate::{
-    context, result_string_init,
+    check_call, context, result_string_init,
     string_return::{callback_get_result_string, callback_get_result_string_data},
 };
 
@@ -10,23 +10,19 @@ pub fn set(key: &str, value: &str) -> Result<()> {
     let mut ctx = context::Context::new();
     let key = std::ffi::CString::new(key)?;
     let value = std::ffi::CString::new(value)?;
-    ctx.check_one_call(|ctx_ptr| unsafe {
+    unsafe {
-        raw::setting_set(ctx_ptr, key.as_ptr(), value.as_ptr());
+        check_call!(raw::setting_set[&mut ctx, key.as_ptr(), value.as_ptr()])?;
-    })
+    }
+    Ok(())
 }
 
 pub fn get(key: &str) -> Result<String> {
     let mut ctx = context::Context::new();
     let key = std::ffi::CString::new(key)?;
     let mut r: Result<String> = result_string_init!();
-    ctx.check_one_call(|ctx_ptr| unsafe {
+    unsafe {
-        raw::setting_get(
+        check_call!(raw::setting_get[&mut ctx, key.as_ptr(), Some(callback_get_result_string), callback_get_result_string_data(&mut r)])?;
-            ctx_ptr,
+    }
-            key.as_ptr(),
-            Some(callback_get_result_string),
-            callback_get_result_string_data(&mut r),
-        )
-    })?;
     r
 }