refact: Make the callback convert to Result<String> 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

@@ -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<String> {
-        let mut raw_buffer: Vec<u8> = Vec::new();
+        let mut r = result_string_init!();
         unsafe {
             raw::get_string(
                 self.context.ptr(),
                 value.raw_ptr(),
-                Some(callback_get_vec_u8),
+                Some(callback_get_result_string),
-                &mut raw_buffer as *mut Vec<u8> as *mut std::ffi::c_void,
+                callback_get_result_string_data(&mut r),
             )
         };
         self.context.check_err()?;
-        String::from_utf8(raw_buffer)
+        r
-            .map_err(|e| anyhow::format_err!("Nix string is not valid UTF-8: {}", e))
     }
     /// 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<String> {

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<String> {
         unsafe {
-            let mut vec = Vec::new();
+            let mut r = result_string_init!();
             raw::store_path_name(
                 self.raw,
-                Some(callback_get_vec_u8),
+                Some(callback_get_result_string),
-                callback_get_vec_u8_data(&mut vec),
+                callback_get_result_string_data(&mut r),
             );
-            String::from_utf8(vec).map_err(|e| e.into())
+            r
         }
     }
 }

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<String> {
-        let mut raw_buffer: Vec<u8> = Vec::new();
+        let mut r = result_string_init!();
         unsafe {
             raw::store_get_uri(
                 self.context.ptr(),
                 self.inner.ptr(),
-                Some(callback_get_vec_u8),
+                Some(callback_get_result_string),
-                callback_get_vec_u8_data(&mut raw_buffer),
+                callback_get_result_string_data(&mut r),
             )
         };
         self.context.check_err()?;
-        String::from_utf8(raw_buffer).map_err(|e| e.into())
+        r
     }
 }
 

rust/nix-util/src/lib.rs

@@ -1,3 +1,4 @@
 pub mod context;
 pub mod settings;
+#[macro_use]
 pub mod string_return;

rust/nix-util/src/settings.rs

@@ -2,8 +2,8 @@ use anyhow::Result;
 use nix_c_raw as raw;
 
 use crate::{
-    context,
+    context, result_string_init,
-    string_return::{callback_get_vec_u8, callback_get_vec_u8_data},
+    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<String> {
     let ctx = context::Context::new();
     let key = std::ffi::CString::new(key)?;
-    let mut raw_buffer: Vec<u8> = Vec::new();
+    let mut r: Result<String> = result_string_init!();
     unsafe {
         raw::setting_get(
             ctx.ptr(),
             key.as_ptr(),
-            Some(callback_get_vec_u8),
+            Some(callback_get_result_string),
-            callback_get_vec_u8_data(&mut raw_buffer),
+            callback_get_result_string_data(&mut r),
         )
     };
     ctx.check_err()?;
-    String::from_utf8(raw_buffer).map_err(|e| e.into())
+    r
 }
 
 #[cfg(test)]

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<u8>;
+    let ret = user_data as *mut Result<String>;
     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<u8>) -> *mut std::os::raw::c_void {
+pub fn callback_get_result_string_data(vec: &mut Result<String>) -> *mut std::os::raw::c_void {
-    vec as *mut Vec<u8> as *mut std::os::raw::c_void
+    vec as *mut Result<String> 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() {
+    fn test_callback_get_result_string_empty() {
-        let mut ret: Vec<u8> = Vec::new();
+        let mut ret: Result<String> = 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 =
+        let user_data: *mut std::os::raw::c_void = callback_get_result_string_data(&mut ret);
-            &mut ret as *mut Vec<u8> as *mut std::os::raw::c_void;
 
         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() {
+    fn test_callback_result_string() {
-        let mut ret: Vec<u8> = Vec::new();
+        let mut ret: Result<String> = 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 =
+        let user_data: *mut std::os::raw::c_void = callback_get_result_string_data(&mut ret);
-            &mut ret as *mut Vec<u8> as *mut std::os::raw::c_void;
-
         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");
     }
 }