aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobin Krahl <robin.krahl@ireas.org>2020-01-28 17:56:53 +0100
committerRobin Krahl <robin.krahl@ireas.org>2020-02-03 11:31:21 +0100
commitd905aafe208536203dd319e5dc48bd250180a8ef (patch)
tree15566bcd7ff23f940a2782e077f8f93a7bd44e84
parent4fb865af37093d9b0ee039d8ae48fb2a820f3760 (diff)
downloadnitrokey-rs-d905aafe208536203dd319e5dc48bd250180a8ef.tar.gz
nitrokey-rs-d905aafe208536203dd319e5dc48bd250180a8ef.tar.bz2
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.
-rw-r--r--src/device/mod.rs7
-rw-r--r--src/util.rs36
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> {