diff --git a/rust/nix-expr/src/value.rs b/rust/nix-expr/src/value.rs index da33c87..83aa26e 100644 --- a/rust/nix-expr/src/value.rs +++ b/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 } } } diff --git a/rust/nix-store/src/store.rs b/rust/nix-store/src/store.rs index ea0812b..f05bd07 100644 --- a/rust/nix-store/src/store.rs +++ b/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 { - let context: Context = Context::new(); - raw::libstore_init(context.ptr()); - context.check_err() - } + Context::new().check_one_call(|ctx_ptr| unsafe { + raw::libstore_init(ctx_ptr); + }) }; } @@ -76,9 +74,9 @@ impl Store { .chain(std::iter::once(null_mut())) // signal the end of the array .collect(); - let store = - unsafe { raw::store_open(context.ptr(), uri_ptr.as_ptr(), params.as_mut_ptr()) }; - context.check_err()?; + let store = context.check_one_call(|ctx_ptr| unsafe { + raw::store_open(ctx_ptr, uri_ptr.as_ptr(), params.as_mut_ptr()) + })?; 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 { 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 } } diff --git a/rust/nix-util/src/context.rs b/rust/nix-util/src/context.rs index 1e8e6e1..657e85a 100644 --- a/rust/nix-util/src/context.rs +++ b/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> { 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 { diff --git a/rust/nix-util/src/settings.rs b/rust/nix-util/src/settings.rs index 36964ad..6b4d0f4 100644 --- a/rust/nix-util/src/settings.rs +++ b/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 { - raw::setting_set(ctx.ptr(), key.as_ptr(), value.as_ptr()); - }; - ctx.check_err() + ctx.check_one_call(|ctx_ptr| unsafe { + raw::setting_set(ctx_ptr, key.as_ptr(), value.as_ptr()); + }) } pub fn get(key: &str) -> Result { let ctx = context::Context::new(); let key = std::ffi::CString::new(key)?; let mut r: Result = 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 { - nix_c_raw::libstore_init(ctx.ptr()); - }; - ctx.check_err().unwrap(); + ctx.check_one_call(|ctx_ptr| unsafe { + nix_c_raw::libstore_init(ctx_ptr); + }) + .unwrap(); } #[test]