From d905aafe208536203dd319e5dc48bd250180a8ef Mon Sep 17 00:00:00 2001
From: Robin Krahl <robin.krahl@ireas.org>
Date: Tue, 28 Jan 2020 17:56:53 +0100
Subject: Refactor string handling in util

The util module provides helper methods to deal with the C strings
returned by libnitrokey.  The current implementation has to problems:
- It causes unnecessary allocations if we only want to look at the
  string, for example in get_serial_number.
- If the conversion from a CStr to a String fails, the string pointer
  is not freed.

Therefore this patch introduces the run_with_str function that executes
a function with the string returned by libnitrokey and then makes sure
that the pointer is freed correctly.
---
 src/device/mod.rs |  7 ++++---
 src/util.rs       | 36 +++++++++++++++++++++++-------------
 2 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/src/device/mod.rs b/src/device/mod.rs
index 83ab90d..067fdf6 100644
--- a/src/device/mod.rs
+++ b/src/device/mod.rs
@@ -18,7 +18,7 @@ use crate::error::{CommunicationError, Error, LibraryError};
 use crate::otp::GenerateOtp;
 use crate::pws::GetPasswordSafe;
 use crate::util::{
-    get_command_result, get_cstring, owned_str_from_ptr, result_from_string, result_or_error,
+    get_command_result, get_cstring, owned_str_from_ptr, result_or_error, run_with_string,
 };
 
 pub use pro::Pro;
@@ -375,8 +375,9 @@ pub trait Device<'a>: Authenticate<'a> + GetPasswordSafe<'a> + GenerateOtp + fmt
     /// # }
     /// ```
     fn get_serial_number(&self) -> Result<SerialNumber, Error> {
-        result_from_string(unsafe { nitrokey_sys::NK_device_serial_number() })
-            .and_then(|s| s.parse())
+        run_with_string(unsafe { nitrokey_sys::NK_device_serial_number() }, |s| {
+            s.parse()
+        })
     }
 
     /// Returns the number of remaining authentication attempts for the user.  The total number of
diff --git a/src/util.rs b/src/util.rs
index 08946d6..b17b071 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -30,28 +30,38 @@ pub enum LogLevel {
     DebugL2,
 }
 
+pub fn str_from_ptr<'a>(ptr: *const c_char) -> Result<&'a str, Error> {
+    unsafe { CStr::from_ptr(ptr) }.to_str().map_err(Error::from)
+}
+
 pub fn owned_str_from_ptr(ptr: *const c_char) -> Result<String, Error> {
-    unsafe { CStr::from_ptr(ptr) }
-        .to_str()
-        .map(String::from)
-        .map_err(Error::from)
+    str_from_ptr(ptr).map(ToOwned::to_owned)
 }
 
-pub fn result_from_string(ptr: *const c_char) -> Result<String, Error> {
+pub fn run_with_string<R, F>(ptr: *const c_char, op: F) -> Result<R, Error>
+where
+    F: FnOnce(&str) -> Result<R, Error>,
+{
     if ptr.is_null() {
         return Err(Error::UnexpectedError(
             "libnitrokey returned a null pointer".to_owned(),
         ));
     }
-    let s = owned_str_from_ptr(ptr)?;
+    let result = str_from_ptr(ptr).and_then(op);
     unsafe { free(ptr as *mut c_void) };
-    // An empty string can both indicate an error or be a valid return value.  In this case, we
-    // have to check the last command status to decide what to return.
-    if s.is_empty() {
-        get_last_result().map(|_| s)
-    } else {
-        Ok(s)
-    }
+    result
+}
+
+pub fn result_from_string(ptr: *const c_char) -> Result<String, Error> {
+    run_with_string(ptr, |s| {
+        // An empty string can both indicate an error or be a valid return value.  In this case, we
+        // have to check the last command status to decide what to return.
+        if s.is_empty() {
+            get_last_result().map(|_| s.to_owned())
+        } else {
+            Ok(s.to_owned())
+        }
+    })
 }
 
 pub fn result_or_error<T>(value: T) -> Result<T, Error> {
-- 
cgit v1.2.3