refact: Clean up Context interface to be safer

The mutation-based methods had some pitfalls, and we don't really
need them.

We could re-add them if we need to.

(cherry picked from commit ca92b8491d87cebf54dd2468599168fc7a16c07f)

rust/nix-expr/src/value.rs

@@ -74,9 +74,10 @@ impl Drop for Value {
 impl Clone for Value {
     fn clone(&self) -> Self {
         let context = Context::new();
-        unsafe { raw::gc_incref(context.ptr(), self.inner.as_ptr()) };
+        context
+            .check_one_call(|ctx_ptr| unsafe { raw::gc_incref(ctx_ptr, 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
-        context.check_err().unwrap();
         Value { inner: self.inner }
     }
 }

rust/nix-store/src/store.rs

@@ -11,11 +11,9 @@ use std::ptr::NonNull;
 /* TODO make Nix itself thread safe */
 lazy_static! {
     static ref INIT: Result<()> = {
-        unsafe {
+        Context::new().check_one_call(|ctx_ptr| unsafe {
-            let context: Context = Context::new();
+            raw::libstore_init(ctx_ptr);
-            raw::libstore_init(context.ptr());
+        })
-            context.check_err()
-        }
     };
 }
 
@@ -76,9 +74,9 @@ impl Store {
             .chain(std::iter::once(null_mut())) // signal the end of the array
             .collect();
 
-        let store =
+        let store = context.check_one_call(|ctx_ptr| unsafe {
-            unsafe { raw::store_open(context.ptr(), uri_ptr.as_ptr(), params.as_mut_ptr()) };
+            raw::store_open(ctx_ptr, uri_ptr.as_ptr(), params.as_mut_ptr())
-        context.check_err()?;
+        })?;
         if store.is_null() {
             panic!("nix_c_store_open returned a null pointer without an error");
         }
@@ -97,15 +95,14 @@ impl Store {
 
     pub fn get_uri(&self) -> Result<String> {
         let mut r = result_string_init!();
-        unsafe {
+        self.context.check_one_call(|ctx_ptr| unsafe {
             raw::store_get_uri(
-                self.context.ptr(),
+                ctx_ptr,
                 self.inner.ptr(),
                 Some(callback_get_result_string),
                 callback_get_result_string_data(&mut r),
             )
-        };
+        })?;
-        self.context.check_err()?;
         r
     }
 }

rust/nix-util/src/context.rs

@@ -20,10 +20,22 @@ impl Context {
         };
         ctx
     }
-    pub fn ptr(&self) -> *mut raw::c_context {
+
+    /// This is currently private because of the switch to `check_one_call`, which is more ergonomic.
+    /// The pattern its callers use make it hard to forget to use this function.
+    ///
+    /// If we have a good use case for `check_err`, we can expose it again.
+    fn ptr(&self) -> *mut raw::c_context {
         self.inner.as_ptr()
     }
-    pub fn check_err(&self) -> Result<()> {
+
+    /// Check the error code and return an error if it's not `NIX_OK`.
+    ///
+    /// This is currently private because of the switch to `check_one_call`, which is more ergonomic.
+    /// The pattern its callers use make it hard to forget to use this function.
+    ///
+    /// If we have a good use case for `check_err`, we can expose it again.
+    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 (pointing into the context), so we don't need to free it
@@ -35,7 +47,7 @@ impl Context {
         Ok(())
     }
 
-    pub fn clear(&self) {
+    fn clear(&self) {
         unsafe {
             raw::set_err_msg(
                 self.inner.as_ptr(),
@@ -67,18 +79,13 @@ impl Context {
         f: F,
     ) -> Result<Option<T>> {
         let t = f(self.ptr());
-        if self.is_key_error() {
+        if unsafe { raw::err_code(self.inner.as_ptr()) == raw::NIX_ERR_KEY } {
             self.clear();
             return Ok(None);
         }
         self.check_err_and_clear()?;
         Ok(Some(t))
     }
-
-    /// NIX_ERR_KEY is returned when e.g. an attribute is missing. Return true if the error is of this type.
-    pub fn is_key_error(&self) -> bool {
-        unsafe { raw::err_code(self.inner.as_ptr()) == raw::NIX_ERR_KEY }
-    }
 }
 
 impl Drop for Context {

rust/nix-util/src/settings.rs

@@ -10,25 +10,23 @@ pub fn set(key: &str, value: &str) -> Result<()> {
     let ctx = context::Context::new();
     let key = std::ffi::CString::new(key)?;
     let value = std::ffi::CString::new(value)?;
-    unsafe {
+    ctx.check_one_call(|ctx_ptr| unsafe {
-        raw::setting_set(ctx.ptr(), key.as_ptr(), value.as_ptr());
+        raw::setting_set(ctx_ptr, key.as_ptr(), value.as_ptr());
-    };
+    })
-    ctx.check_err()
 }
 
 pub fn get(key: &str) -> Result<String> {
     let ctx = context::Context::new();
     let key = std::ffi::CString::new(key)?;
     let mut r: Result<String> = result_string_init!();
-    unsafe {
+    ctx.check_one_call(|ctx_ptr| unsafe {
         raw::setting_get(
-            ctx.ptr(),
+            ctx_ptr,
             key.as_ptr(),
             Some(callback_get_result_string),
             callback_get_result_string_data(&mut r),
         )
-    };
+    })?;
-    ctx.check_err()?;
     r
 }
 
@@ -39,10 +37,10 @@ mod tests {
     #[ctor::ctor]
     fn setup() {
         let ctx = context::Context::new();
-        unsafe {
+        ctx.check_one_call(|ctx_ptr| unsafe {
-            nix_c_raw::libstore_init(ctx.ptr());
+            nix_c_raw::libstore_init(ctx_ptr);
-        };
+        })
-        ctx.check_err().unwrap();
+        .unwrap();
     }
 
     #[test]