From 4688ccbf953566fd77e4ecc2e46c451a69e53d0c Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Wed, 22 May 2024 14:45:12 +0200 Subject: [PATCH] refact: Make the callback convert to Result immediately This is slightly easier to use than the previous pattern that was always followed up by the same conversions. (cherry picked from commit 756c080730cd4fa81d4c0e3a99688cbe8debe57f) --- rust/nix-expr/src/eval_state.rs | 12 +++---- rust/nix-store/src/path.rs | 13 ++++--- rust/nix-store/src/store.rs | 11 +++--- rust/nix-util/src/lib.rs | 1 + rust/nix-util/src/settings.rs | 12 +++---- rust/nix-util/src/string_return.rs | 55 ++++++++++++++++++------------ 6 files changed, 61 insertions(+), 43 deletions(-) diff --git a/rust/nix-expr/src/eval_state.rs b/rust/nix-expr/src/eval_state.rs index c929278..6ff4d54 100644 --- a/rust/nix-expr/src/eval_state.rs +++ b/rust/nix-expr/src/eval_state.rs @@ -6,7 +6,8 @@ use nix_c_raw as raw; use nix_store::path::StorePath; use nix_store::store::Store; use nix_util::context::Context; -use nix_util::string_return::callback_get_vec_u8; +use nix_util::result_string_init; +use nix_util::string_return::{callback_get_result_string, callback_get_result_string_data}; use std::ffi::CString; use std::ptr::null_mut; use std::ptr::NonNull; @@ -116,18 +117,17 @@ impl EvalState { } /// Not exposed, because the caller must always explicitly handle the context or not accept one at all. fn get_string(&self, value: &Value) -> Result { - let mut raw_buffer: Vec = Vec::new(); + let mut r = result_string_init!(); unsafe { raw::get_string( self.context.ptr(), value.raw_ptr(), - Some(callback_get_vec_u8), - &mut raw_buffer as *mut Vec as *mut std::ffi::c_void, + Some(callback_get_result_string), + callback_get_result_string_data(&mut r), ) }; self.context.check_err()?; - String::from_utf8(raw_buffer) - .map_err(|e| anyhow::format_err!("Nix string is not valid UTF-8: {}", e)) + r } /// NOTE: this will be replaced by two methods, one that also returns the context, and one that checks that the context is empty pub fn require_string(&self, value: &Value) -> Result { diff --git a/rust/nix-store/src/path.rs b/rust/nix-store/src/path.rs index f8ca699..17e136d 100644 --- a/rust/nix-store/src/path.rs +++ b/rust/nix-store/src/path.rs @@ -1,6 +1,9 @@ use anyhow::Result; use nix_c_raw as raw; -use nix_util::string_return::{callback_get_vec_u8, callback_get_vec_u8_data}; +use nix_util::{ + result_string_init, + string_return::{callback_get_result_string, callback_get_result_string_data}, +}; pub struct StorePath { raw: *mut raw::StorePath, @@ -25,13 +28,13 @@ impl StorePath { } pub fn name(&self) -> Result { unsafe { - let mut vec = Vec::new(); + let mut r = result_string_init!(); raw::store_path_name( self.raw, - Some(callback_get_vec_u8), - callback_get_vec_u8_data(&mut vec), + Some(callback_get_result_string), + callback_get_result_string_data(&mut r), ); - String::from_utf8(vec).map_err(|e| e.into()) + r } } } diff --git a/rust/nix-store/src/store.rs b/rust/nix-store/src/store.rs index 3aeaa48..98fc79d 100644 --- a/rust/nix-store/src/store.rs +++ b/rust/nix-store/src/store.rs @@ -2,7 +2,8 @@ use anyhow::{bail, Result}; use lazy_static::lazy_static; use nix_c_raw as raw; use nix_util::context::Context; -use nix_util::string_return::{callback_get_vec_u8, callback_get_vec_u8_data}; +use nix_util::result_string_init; +use nix_util::string_return::{callback_get_result_string, callback_get_result_string_data}; use std::ffi::CString; use std::ptr::null_mut; use std::ptr::NonNull; @@ -78,17 +79,17 @@ impl Store { } pub fn get_uri(&self) -> Result { - let mut raw_buffer: Vec = Vec::new(); + let mut r = result_string_init!(); unsafe { raw::store_get_uri( self.context.ptr(), self.inner.ptr(), - Some(callback_get_vec_u8), - callback_get_vec_u8_data(&mut raw_buffer), + Some(callback_get_result_string), + callback_get_result_string_data(&mut r), ) }; self.context.check_err()?; - String::from_utf8(raw_buffer).map_err(|e| e.into()) + r } } diff --git a/rust/nix-util/src/lib.rs b/rust/nix-util/src/lib.rs index e1a943b..29ad7dd 100644 --- a/rust/nix-util/src/lib.rs +++ b/rust/nix-util/src/lib.rs @@ -1,3 +1,4 @@ pub mod context; pub mod settings; +#[macro_use] pub mod string_return; diff --git a/rust/nix-util/src/settings.rs b/rust/nix-util/src/settings.rs index 10f05cc..36964ad 100644 --- a/rust/nix-util/src/settings.rs +++ b/rust/nix-util/src/settings.rs @@ -2,8 +2,8 @@ use anyhow::Result; use nix_c_raw as raw; use crate::{ - context, - string_return::{callback_get_vec_u8, callback_get_vec_u8_data}, + context, result_string_init, + string_return::{callback_get_result_string, callback_get_result_string_data}, }; pub fn set(key: &str, value: &str) -> Result<()> { @@ -19,17 +19,17 @@ pub fn set(key: &str, value: &str) -> Result<()> { pub fn get(key: &str) -> Result { let ctx = context::Context::new(); let key = std::ffi::CString::new(key)?; - let mut raw_buffer: Vec = Vec::new(); + let mut r: Result = result_string_init!(); unsafe { raw::setting_get( ctx.ptr(), key.as_ptr(), - Some(callback_get_vec_u8), - callback_get_vec_u8_data(&mut raw_buffer), + Some(callback_get_result_string), + callback_get_result_string_data(&mut r), ) }; ctx.check_err()?; - String::from_utf8(raw_buffer).map_err(|e| e.into()) + r } #[cfg(test)] diff --git a/rust/nix-util/src/string_return.rs b/rust/nix-util/src/string_return.rs index cfead12..004f1ae 100644 --- a/rust/nix-util/src/string_return.rs +++ b/rust/nix-util/src/string_return.rs @@ -1,23 +1,37 @@ +use anyhow::Result; + /// 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( +pub unsafe extern "C" fn callback_get_result_string( start: *const ::std::os::raw::c_char, n: std::os::raw::c_uint, user_data: *mut std::os::raw::c_void, ) { - let ret = user_data as *mut Vec; + let ret = user_data as *mut Result; let slice = std::slice::from_raw_parts(start as *const u8, n as usize); - if !(*ret).is_empty() { - panic!("callback_get_vec_u8: slice must be empty. Were we called twice?"); + + if !(*ret).is_err() { + panic!( + "callback_get_result_string: Result must be initialized to Err. Did Nix call us twice?" + ); } - (*ret).extend_from_slice(slice); + + *ret = String::from_utf8(slice.to_vec()) + .map_err(|e| anyhow::format_err!("Nix string is not valid UTF-8: {}", e)); } -pub fn callback_get_vec_u8_data(vec: &mut Vec) -> *mut std::os::raw::c_void { - vec as *mut Vec as *mut std::os::raw::c_void +pub fn callback_get_result_string_data(vec: &mut Result) -> *mut std::os::raw::c_void { + vec as *mut Result as *mut std::os::raw::c_void +} + +#[macro_export] +macro_rules! result_string_init { + () => { + Err(anyhow::anyhow!("String was not set by Nix C API")) + }; } #[cfg(test)] @@ -26,35 +40,34 @@ mod tests { use nix_c_raw as raw; /// Typecheck the function signature against the generated bindings in nix_c_raw. - static _CALLBACK_GET_VEC_U8: raw::get_string_callback = Some(callback_get_vec_u8); + static _CALLBACK_GET_RESULT_STRING: raw::get_string_callback = Some(callback_get_result_string); #[test] - fn test_callback_get_vec_u8_empty() { - let mut ret: Vec = Vec::new(); + fn test_callback_get_result_string_empty() { + let mut ret: Result = result_string_init!(); let start: *const std::os::raw::c_char = std::ptr::null(); let n: std::os::raw::c_uint = 0; - let user_data: *mut std::os::raw::c_void = - &mut ret as *mut Vec as *mut std::os::raw::c_void; + let user_data: *mut std::os::raw::c_void = callback_get_result_string_data(&mut ret); unsafe { - callback_get_vec_u8(start, n, user_data); + callback_get_result_string(start, n, user_data); } - assert_eq!(ret, vec![]); + let s = ret.unwrap(); + assert_eq!(s, ""); } #[test] - fn test_callback_get_vec_u8() { - let mut ret: Vec = Vec::new(); + fn test_callback_result_string() { + let mut ret: Result = result_string_init!(); let start: *const std::os::raw::c_char = b"helloGARBAGE".as_ptr() as *const i8; let n: std::os::raw::c_uint = 5; - let user_data: *mut std::os::raw::c_void = - &mut ret as *mut Vec as *mut std::os::raw::c_void; - + let user_data: *mut std::os::raw::c_void = callback_get_result_string_data(&mut ret); unsafe { - callback_get_vec_u8(start, n, user_data); + callback_get_result_string(start, n, user_data); } - assert_eq!(ret, b"hello".to_vec()); + let s = ret.unwrap(); + assert_eq!(s, "hello"); } }