Represent serial numbers using SerialNumber struct

In a previous commit, we changed the serial number representation from a
string to an integer.  This made it easier to compare serial numbers,
but also introduced new problems:
- Serial numbers should be formatted consistently, for example as
  "{:#010x}".  It is hard to ensure this for an integer value.
- The format of the serial number may be subject to change.  Users
  should not rely too much on the u32 representation.

Therefore we introduce a new SerialNumber struct that represents a
serial number.  Currently it only stores a u32 value.  The following
traits and functions can be used to access its value:
- FromStr for string parsing
- ToString/Display for string formatting
- as_u32 to access the underlying integer value

6 files changed, 174 insertions, 65 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c94e170..9e675f0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,11 +5,12 @@ SPDX-License-Identifier: CC0-1.0
 
 # Unreleased
 - Add `String` value to the `Error::UnexpectedError` variant.
-- Always store serial numbers as integers:
+- Always store serial numbers as structs:
+  - Introduce the `SerialNumber` struct.
   - Change the return type of `Device::get_serial_number` from `Result<String,
-    _>` to `Result<u32, _>`.
+    _>` to `Result<SerialNumber, _>`.
   - Change the type of the field `DeviceInfo.serial_number` from
-    `Option<String>` to `Option<u32>`.
+    `Option<String>` to `Option<SerialNumber>`.
 - Use `NK_get_status` instead of `NK_read_config` to implement the
   `Device::get_config` function.
 
diff --git a/examples/list-devices.rs b/examples/list-devices.rs
index 47fa054..0066f8c 100644
--- a/examples/list-devices.rs
+++ b/examples/list-devices.rs
@@ -17,7 +17,7 @@ fn main() -> Result<(), nitrokey::Error> {
             let model = device.get_model();
             let status = device.get_status()?;
             println!(
-                "{}\t{}\t{}\t\t\t{:08x}",
+                "{}\t{}\t{}\t\t\t{}",
                 device_info.path, model, status.firmware_version, status.serial_number
             );
         }
diff --git a/src/device/mod.rs b/src/device/mod.rs
index e9ca0dc..83ab90d 100644
--- a/src/device/mod.rs
+++ b/src/device/mod.rs
@@ -8,12 +8,13 @@ mod wrapper;
 use std::convert::{TryFrom, TryInto};
 use std::ffi;
 use std::fmt;
+use std::str;
 
 use nitrokey_sys;
 
 use crate::auth::Authenticate;
 use crate::config::{Config, RawConfig};
-use crate::error::{CommunicationError, Error};
+use crate::error::{CommunicationError, Error, LibraryError};
 use crate::otp::GenerateOtp;
 use crate::pws::GetPasswordSafe;
 use crate::util::{
@@ -69,6 +70,98 @@ impl TryFrom<nitrokey_sys::NK_device_model> for Model {
     }
 }
 
+/// Serial number of a Nitrokey device.
+///
+/// The serial number can be formatted as a string using the [`ToString`][] trait, and it can be
+/// parsed from a string using the [`FromStr`][] trait.  It can also be represented as a 32-bit
+/// unsigned integer using [`as_u32`][].  This integer is the ID of the smartcard of the Nitrokey
+/// device.
+///
+/// Neither the format of the string representation nor the integer representation are guaranteed
+/// to stay the same for new firmware versions.
+///
+/// [`as_u32`]: #method.as_u32
+/// [`FromStr`]: #impl-FromStr
+/// [`ToString`]: #impl-ToString
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub struct SerialNumber {
+    value: u32,
+}
+
+impl SerialNumber {
+    /// Creates an emtpty serial number.
+    ///
+    /// This function can be used to create a placeholder value or to compare a `SerialNumber`
+    /// instance with an empty serial number.
+    pub fn empty() -> Self {
+        SerialNumber::new(0)
+    }
+
+    fn new(value: u32) -> Self {
+        SerialNumber { value }
+    }
+
+    /// Returns the integer reprensentation of this serial number.
+    ///
+    /// This integer currently is the ID of the smartcard of the Nitrokey device.  Upcoming
+    /// firmware versions might change the meaning of this representation, or add additional
+    /// components to the serial number.
+    // To provide a stable API even if the internal representation of SerialNumber changes, we want
+    // to borrow SerialNumber instead of copying it even if it might be less efficient.
+    #[allow(clippy::trivially_copy_pass_by_ref)]
+    pub fn as_u32(&self) -> u32 {
+        self.value
+    }
+}
+
+impl fmt::Display for SerialNumber {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{:#010x}", self.value)
+    }
+}
+
+impl str::FromStr for SerialNumber {
+    type Err = Error;
+
+    /// Try to parse a serial number from a hex string.
+    ///
+    /// The input string must be a valid hex string.  Optionally, it can include a `0x` prefix.
+    ///
+    /// # Errors
+    ///
+    /// - [`InvalidHexString`][] if the given string is not a valid hex string
+    ///
+    /// # Example
+    ///
+    /// ```no_run
+    /// use std::convert::TryFrom;
+    /// use nitrokey::{DeviceInfo, Error, SerialNumber};
+    ///
+    /// fn find_device(serial_number: &str) -> Result<Option<DeviceInfo>, Error> {
+    ///     let serial_number: SerialNumber = serial_number.parse()?;
+    ///     Ok(nitrokey::list_devices()?
+    ///         .into_iter()
+    ///         .filter(|device| device.serial_number == Some(serial_number))
+    ///         .next())
+    /// }
+    ///
+    /// ```
+    ///
+    /// [`InvalidHexString`]: enum.LibraryError.html#variant.InvalidHexString
+    fn from_str(s: &str) -> Result<SerialNumber, Error> {
+        // ignore leading 0x
+        let hex_string = if s.starts_with("0x") {
+            s.split_at(2).1
+        } else {
+            s
+        };
+
+        u32::from_str_radix(hex_string, 16)
+            .map(SerialNumber::new)
+            .map_err(|_| LibraryError::InvalidHexString.into())
+    }
+}
+
 /// Connection information for a Nitrokey device.
 #[derive(Clone, Debug, PartialEq)]
 pub struct DeviceInfo {
@@ -77,7 +170,7 @@ pub struct DeviceInfo {
     /// The USB device path.
     pub path: String,
     /// The serial number of the device, or `None` if the device does not expose its serial number.
-    pub serial_number: Option<u32>,
+    pub serial_number: Option<SerialNumber>,
 }
 
 impl TryFrom<&nitrokey_sys::NK_device_info> for DeviceInfo {
@@ -108,19 +201,13 @@ impl fmt::Display for DeviceInfo {
         }
         write!(f, " at {} with ", self.path)?;
         match self.serial_number {
-            Some(serial_number) => write!(f, "serial no. 0x{:08x}", serial_number),
+            Some(serial_number) => write!(f, "serial no. {}", serial_number),
             None => write!(f, "an unknown serial number"),
         }
     }
 }
 
-/// Parses the given hex string and returns its integer representation.
-fn parse_serial_number<S: AsRef<str>>(s: S) -> Result<u32, Error> {
-    u32::from_str_radix(s.as_ref(), 16)
-        .map_err(|_| Error::UnexpectedError("Could not parse hex string".to_owned()))
-}
-
-/// Parses a serial number returned by hidapi and returns its integer value.
+/// Parses a serial number returned by hidapi.
 ///
 /// If the serial number is all zero, this function returns `None`.  Otherwise, it uses the last
 /// eight characters.  If these are all zero, the first eight characters are used instead.  The
@@ -131,7 +218,7 @@ fn parse_serial_number<S: AsRef<str>>(s: S) -> Result<u32, Error> {
 /// all (all zero value), while the Nitrokey Pro with firmware 0.9 or later writes its serial
 /// number to the last eight characters.  Nitrokey Pro devices with firmware 0.8 or earlier wrote
 /// their serial number to the first eight characters.
-fn get_hidapi_serial_number(serial_number: &str) -> Option<u32> {
+fn get_hidapi_serial_number(serial_number: &str) -> Option<SerialNumber> {
     let len = serial_number.len();
     if len < 8 {
         // The serial number in the USB descriptor has 12 bytes, we need at least four
@@ -148,7 +235,7 @@ fn get_hidapi_serial_number(serial_number: &str) -> Option<u32> {
             // The last eight characters are all zero --> use the first eight
             serial_number.split_at(8).0
         };
-        parse_serial_number(substr).ok()
+        substr.parse().ok()
     } else {
         // The serial number is all zero
         None
@@ -176,7 +263,7 @@ pub struct Status {
     /// The firmware version of the device.
     pub firmware_version: FirmwareVersion,
     /// The serial number of the device.
-    pub serial_number: u32,
+    pub serial_number: SerialNumber,
     /// The configuration of the device.
     pub config: Config,
 }
@@ -188,7 +275,7 @@ impl From<nitrokey_sys::NK_status> for Status {
                 major: status.firmware_version_major,
                 minor: status.firmware_version_minor,
             },
-            serial_number: status.serial_number_smart_card,
+            serial_number: SerialNumber::new(status.serial_number_smart_card),
             config: RawConfig::from(&status).into(),
         }
     }
@@ -257,7 +344,7 @@ pub trait Device<'a>: Authenticate<'a> + GetPasswordSafe<'a> + GenerateOtp + fmt
     /// let device = manager.connect()?;
     /// let status = device.get_status()?;
     /// println!("Firmware version: {}", status.firmware_version);
-    /// println!("Serial number:    0x{:08x}", status.serial_number);
+    /// println!("Serial number:    {}", status.serial_number);
     /// # Ok::<(), nitrokey::Error>(())
     /// ```
     ///
@@ -281,15 +368,15 @@ pub trait Device<'a>: Authenticate<'a> + GetPasswordSafe<'a> + GenerateOtp + fmt
     /// let mut manager = nitrokey::take()?;
     /// let device = manager.connect()?;
     /// match device.get_serial_number() {
-    ///     Ok(number) => println!("serial no: 0x{:08x}", number),
+    ///     Ok(number) => println!("serial no: {}", number),
     ///     Err(err) => eprintln!("Could not get serial number: {}", err),
     /// };
     /// #     Ok(())
     /// # }
     /// ```
-    fn get_serial_number(&self) -> Result<u32, Error> {
+    fn get_serial_number(&self) -> Result<SerialNumber, Error> {
         result_from_string(unsafe { nitrokey_sys::NK_device_serial_number() })
-            .and_then(parse_serial_number)
+            .and_then(|s| s.parse())
     }
 
     /// Returns the number of remaining authentication attempts for the user.  The total number of
@@ -624,26 +711,50 @@ pub(crate) fn connect_enum(model: Model) -> bool {
 
 #[cfg(test)]
 mod tests {
-    use super::{get_hidapi_serial_number, parse_serial_number};
+    use std::str::FromStr;
+
+    use super::{get_hidapi_serial_number, LibraryError, SerialNumber};
+
+    #[test]
+    fn test_serial_number_display() {
+        fn assert_str(s: &str, n: u32) {
+            assert_eq!(s.to_owned(), SerialNumber::new(n).to_string());
+        }
+
+        assert_str("0x00000000", 0);
+        assert_str("0x00001000", 0x1000);
+        assert_str("0x12345678", 0x12345678);
+    }
 
     #[test]
-    fn test_parse_serial_number() {
+    fn test_serial_number_try_from() {
+        fn assert_ok(v: u32, s: &str) {
+            assert_eq!(SerialNumber::new(v), SerialNumber::from_str(s).unwrap());
+            assert_eq!(
+                SerialNumber::new(v),
+                SerialNumber::from_str(format!("0x{}", s).as_ref()).unwrap()
+            );
+        }
+
         fn assert_err(s: &str) {
-            match parse_serial_number(s).unwrap_err() {
-                super::Error::UnexpectedError(_) => {}
-                err => assert!(false, "expected UnexpectedError, got {} (input {})", err, s),
+            match SerialNumber::from_str(s).unwrap_err() {
+                super::Error::LibraryError(LibraryError::InvalidHexString) => {}
+                err => assert!(
+                    false,
+                    "expected InvalidHexString error, got {} (input {})",
+                    err, s
+                ),
             }
         }
 
-        assert_eq!(0x1234, parse_serial_number("1234").unwrap());
-        assert_eq!(0x1234, parse_serial_number("01234").unwrap());
-        assert_eq!(0x1234, parse_serial_number("001234").unwrap());
-        assert_eq!(0x1234, parse_serial_number("0001234").unwrap());
+        assert_ok(0x1234, "1234");
+        assert_ok(0x1234, "01234");
+        assert_ok(0x1234, "001234");
+        assert_ok(0x1234, "0001234");
 
-        assert_eq!(0, parse_serial_number("0").unwrap());
+        assert_ok(0, "0");
+        assert_ok(0xdeadbeef, "deadbeef");
 
-        assert_eq!(0xdeadbeef, parse_serial_number("deadbeef").unwrap());
-        assert_err("0xdeadbeef");
         assert_err("deadpork");
         assert_err("blubb");
         assert_err("");
@@ -651,29 +762,26 @@ mod tests {
 
     #[test]
     fn test_get_hidapi_serial_number() {
-        assert_eq!(None, get_hidapi_serial_number(""));
-        assert_eq!(None, get_hidapi_serial_number("00000000000000000"));
-        assert_eq!(None, get_hidapi_serial_number("blubb"));
-        assert_eq!(None, get_hidapi_serial_number("1234"));
-        assert_eq!(Some(0x1234), get_hidapi_serial_number("00001234"));
-        assert_eq!(Some(0x1234), get_hidapi_serial_number("000000001234"));
-        assert_eq!(Some(0x1234), get_hidapi_serial_number("100000001234"));
-        assert_eq!(Some(0x12340000), get_hidapi_serial_number("123400000000"));
-        assert_eq!(
-            Some(0x5678),
-            get_hidapi_serial_number("000000000000000000005678")
-        );
-        assert_eq!(
-            Some(0x1234),
-            get_hidapi_serial_number("000012340000000000000000")
-        );
-        assert_eq!(
-            Some(0xffff),
-            get_hidapi_serial_number("00000000000000000000FFFF")
-        );
-        assert_eq!(
-            Some(0xffff),
-            get_hidapi_serial_number("00000000000000000000ffff")
-        );
+        fn assert_none(s: &str) {
+            assert_eq!(None, get_hidapi_serial_number(s));
+        }
+
+        fn assert_some(n: u32, s: &str) {
+            assert_eq!(Some(SerialNumber::new(n)), get_hidapi_serial_number(s));
+        }
+
+        assert_none("");
+        assert_none("00000000000000000");
+        assert_none("blubb");
+        assert_none("1234");
+
+        assert_some(0x1234, "00001234");
+        assert_some(0x1234, "000000001234");
+        assert_some(0x1234, "100000001234");
+        assert_some(0x12340000, "123400000000");
+        assert_some(0x5678, "000000000000000000005678");
+        assert_some(0x1234, "000012340000000000000000");
+        assert_some(0xffff, "00000000000000000000FFFF");
+        assert_some(0xffff, "00000000000000000000ffff");
     }
 }
diff --git a/src/device/storage.rs b/src/device/storage.rs
index 1e2c46d..5669a91 100644
--- a/src/device/storage.rs
+++ b/src/device/storage.rs
@@ -7,7 +7,7 @@ use std::ops;
 
 use nitrokey_sys;
 
-use crate::device::{Device, FirmwareVersion, Model, Status};
+use crate::device::{Device, FirmwareVersion, Model, SerialNumber, Status};
 use crate::error::{CommandError, Error};
 use crate::otp::GenerateOtp;
 use crate::util::{get_command_result, get_cstring, get_last_error};
@@ -827,7 +827,7 @@ impl<'a> Device<'a> for Storage<'a> {
 
         let storage_status = self.get_storage_status()?;
         status.firmware_version = storage_status.firmware_version;
-        status.serial_number = storage_status.serial_number_smart_card;
+        status.serial_number = SerialNumber::new(storage_status.serial_number_smart_card);
 
         Ok(status)
     }
diff --git a/src/lib.rs b/src/lib.rs
index 9efad91..92247d7 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -138,8 +138,8 @@ use nitrokey_sys;
 pub use crate::auth::{Admin, Authenticate, User};
 pub use crate::config::Config;
 pub use crate::device::{
-    Device, DeviceInfo, DeviceWrapper, Model, OperationStatus, Pro, SdCardData, Status, Storage,
-    StorageProductionInfo, StorageStatus, VolumeMode, VolumeStatus,
+    Device, DeviceInfo, DeviceWrapper, Model, OperationStatus, Pro, SdCardData, SerialNumber,
+    Status, Storage, StorageProductionInfo, StorageStatus, VolumeMode, VolumeStatus,
 };
 pub use crate::error::{CommandError, CommunicationError, Error, LibraryError};
 pub use crate::otp::{ConfigureOtp, GenerateOtp, OtpMode, OtpSlotData};
diff --git a/tests/device.rs b/tests/device.rs
index 2989941..1669d9d 100644
--- a/tests/device.rs
+++ b/tests/device.rs
@@ -10,7 +10,7 @@ use std::{thread, time};
 use nitrokey::{
     Authenticate, CommandError, CommunicationError, Config, ConfigureOtp, Device, DeviceInfo,
     Error, GenerateOtp, GetPasswordSafe, LibraryError, OperationStatus, OtpMode, OtpSlotData,
-    Storage, VolumeMode, DEFAULT_ADMIN_PIN, DEFAULT_USER_PIN,
+    SerialNumber, Storage, VolumeMode, DEFAULT_ADMIN_PIN, DEFAULT_USER_PIN,
 };
 use nitrokey_test::test as test_device;
 
@@ -47,7 +47,7 @@ fn list_devices(_device: DeviceWrapper) {
                 nitrokey::Model::Pro => {
                     assert!(device.serial_number.is_some());
                     let serial_number = device.serial_number.unwrap();
-                    assert!(serial_number != 0);
+                    assert!(serial_number != SerialNumber::empty());
                 }
                 nitrokey::Model::Storage => {
                     assert_eq!(None, device.serial_number);
@@ -163,7 +163,7 @@ fn get_status(device: DeviceWrapper) {
 #[test_device]
 fn get_serial_number(device: DeviceWrapper) {
     let serial_number = unwrap_ok!(device.get_serial_number());
-    assert!(serial_number != 0);
+    assert!(serial_number != SerialNumber::empty());
 }
 
 #[test_device]