From c30cbd35ba187cd6e5055d3beb8420b11fb030ec Mon Sep 17 00:00:00 2001
From: Robin Krahl <robin.krahl@ireas.org>
Date: Sun, 27 Jan 2019 23:23:00 +0000
Subject: Always return a Result when communicating with a device

Previously, we sometimes returned a value without wrapping it in a
result if the API method did not indicate errors in the return value.
But we can detect errors using the NK_get_last_command_status function.
This patch changes the return types of these methods to Result<_, Error>
and adds error checks.
---
 src/device.rs | 39 +++++++++++++++++++++++----------------
 src/util.rs   |  4 ++++
 2 files changed, 27 insertions(+), 16 deletions(-)

(limited to 'src')

diff --git a/src/device.rs b/src/device.rs
index 386ce94..4178922 100644
--- a/src/device.rs
+++ b/src/device.rs
@@ -12,7 +12,9 @@ use crate::config::{Config, RawConfig};
 use crate::error::{CommunicationError, Error};
 use crate::otp::GenerateOtp;
 use crate::pws::GetPasswordSafe;
-use crate::util::{get_command_result, get_cstring, get_last_error, result_from_string};
+use crate::util::{
+    get_command_result, get_cstring, get_last_error, result_from_string, result_or_error,
+};
 
 /// Available Nitrokey models.
 #[derive(Clone, Copy, Debug, PartialEq)]
@@ -343,13 +345,15 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp + fmt::Debug {
     ///
     /// # fn try_main() -> Result<(), Error> {
     /// let device = nitrokey::connect()?;
-    /// let count = device.get_user_retry_count();
-    /// println!("{} remaining authentication attempts (user)", count);
+    /// match device.get_user_retry_count() {
+    ///     Ok(count) => println!("{} remaining authentication attempts (user)", count),
+    ///     Err(err) => println!("Could not get user retry count: {}", err),
+    /// }
     /// #     Ok(())
     /// # }
     /// ```
-    fn get_user_retry_count(&self) -> u8 {
-        unsafe { nitrokey_sys::NK_get_user_retry_count() }
+    fn get_user_retry_count(&self) -> Result<u8, Error> {
+        result_or_error(unsafe { nitrokey_sys::NK_get_user_retry_count() })
     }
 
     /// Returns the number of remaining authentication attempts for the admin.  The total number of
@@ -364,12 +368,15 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp + fmt::Debug {
     /// # fn try_main() -> Result<(), Error> {
     /// let device = nitrokey::connect()?;
     /// let count = device.get_admin_retry_count();
-    /// println!("{} remaining authentication attempts (admin)", count);
+    /// match device.get_admin_retry_count() {
+    ///     Ok(count) => println!("{} remaining authentication attempts (admin)", count),
+    ///     Err(err) => println!("Could not get admin retry count: {}", err),
+    /// }
     /// #     Ok(())
     /// # }
     /// ```
-    fn get_admin_retry_count(&self) -> u8 {
-        unsafe { nitrokey_sys::NK_get_admin_retry_count() }
+    fn get_admin_retry_count(&self) -> Result<u8, Error> {
+        result_or_error(unsafe { nitrokey_sys::NK_get_admin_retry_count() })
     }
 
     /// Returns the major part of the firmware version (should be zero).
@@ -384,14 +391,14 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp + fmt::Debug {
     /// let device = nitrokey::connect()?;
     /// println!(
     ///     "Firmware version: {}.{}",
-    ///     device.get_major_firmware_version(),
-    ///     device.get_minor_firmware_version(),
+    ///     device.get_major_firmware_version().unwrap(),
+    ///     device.get_minor_firmware_version().unwrap(),
     /// );
     /// #     Ok(())
     /// # }
     /// ```
-    fn get_major_firmware_version(&self) -> i32 {
-        unsafe { nitrokey_sys::NK_get_major_firmware_version() }
+    fn get_major_firmware_version(&self) -> Result<i32, Error> {
+        result_or_error(unsafe { nitrokey_sys::NK_get_major_firmware_version() })
     }
 
     /// Returns the minor part of the firmware version (for example 8 for version 0.8).
@@ -406,13 +413,13 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp + fmt::Debug {
     /// let device = nitrokey::connect()?;
     /// println!(
     ///     "Firmware version: {}.{}",
-    ///     device.get_major_firmware_version(),
-    ///     device.get_minor_firmware_version(),
+    ///     device.get_major_firmware_version().unwrap(),
+    ///     device.get_minor_firmware_version().unwrap(),
     /// );
     /// #     Ok(())
     /// # }
-    fn get_minor_firmware_version(&self) -> i32 {
-        unsafe { nitrokey_sys::NK_get_minor_firmware_version() }
+    fn get_minor_firmware_version(&self) -> Result<i32, Error> {
+        result_or_error(unsafe { nitrokey_sys::NK_get_minor_firmware_version() })
     }
 
     /// Returns the current configuration of the Nitrokey device.
diff --git a/src/util.rs b/src/util.rs
index b7e8cd3..fdb73c3 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -53,6 +53,10 @@ pub fn result_from_string(ptr: *const c_char) -> Result<String, Error> {
     }
 }
 
+pub fn result_or_error<T>(value: T) -> Result<T, Error> {
+    get_last_result().and(Ok(value))
+}
+
 pub fn get_command_result(value: c_int) -> Result<(), Error> {
     if value == 0 {
         Ok(())
-- 
cgit v1.2.3