From a47ed53e1a67d1074e2f968b6d472914a9e8b5fa Mon Sep 17 00:00:00 2001
From: Robin Krahl <robin@nitrokey.com>
Date: Wed, 3 Jul 2024 18:58:08 +0200
Subject: [PATCH] Only handle error values in Error and ErrorKind

littlefs2 error codes are negative.  Non-negative values indicate
success.  This patch updates our error types to enforce this property by
changing the return type of Error::new to Option<Self> and by removing
ErrorKind::Success.
---
 CHANGELOG.md |  1 +
 src/io.rs    | 31 ++++++++++++++++++-------------
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 70f1a5ae7..5ad22fb76 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -31,6 +31,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Refactor error types:
   - Rename `Error` enum to `ErrorKind` and make it non-exhaustive.
   - Add `Error` struct that represents an error code that may or may not map to an `ErrorKind`.
+  - Remove `ErrorKind::Success` and enforce negative values error codes for `Error`.
 
 ### Removed
 
diff --git a/src/io.rs b/src/io.rs
index c457539ed..91428a4ec 100644
--- a/src/io.rs
+++ b/src/io.rs
@@ -124,14 +124,20 @@ pub struct Error {
 
 impl Error {
     /// Construct an `Error` from an error code.
-    pub const fn new(code: c_int) -> Self {
-        Self { code }
+    ///
+    /// Error codes that are greater or equals to zero represent success.  In this case, `None` is
+    /// returned.
+    pub const fn new(code: c_int) -> Option<Self> {
+        if code >= 0 {
+            None
+        } else {
+            Some(Self { code })
+        }
     }
 
-    /// Construct an `Error` from an `ErrorKind`.
+    /// Construct an `Error` from an error kind.
     pub const fn from_kind(kind: ErrorKind) -> Self {
         let code = match kind {
-            ErrorKind::Success => ll::lfs_error_LFS_ERR_OK,
             ErrorKind::Io => ll::lfs_error_LFS_ERR_IO,
             ErrorKind::Corruption => ll::lfs_error_LFS_ERR_CORRUPT,
             ErrorKind::NoSuchEntry => ll::lfs_error_LFS_ERR_NOENT,
@@ -147,7 +153,11 @@ impl Error {
             ErrorKind::NoAttribute => ll::lfs_error_LFS_ERR_NOATTR,
             ErrorKind::FilenameTooLong => ll::lfs_error_LFS_ERR_NAMETOOLONG,
         };
-        Self::new(code)
+        if let Some(error) = Self::new(code) {
+            error
+        } else {
+            panic!("error code must be negative");
+        }
     }
 
     /// Return the error code of this error.
@@ -162,8 +172,6 @@ impl Error {
     /// this method returning `None`.
     pub const fn kind(&self) -> Option<ErrorKind> {
         let kind = match self.code {
-            n if n >= 0 => ErrorKind::Success,
-            // negative codes
             ll::lfs_error_LFS_ERR_IO => ErrorKind::Io,
             ll::lfs_error_LFS_ERR_CORRUPT => ErrorKind::Corruption,
             ll::lfs_error_LFS_ERR_NOENT => ErrorKind::NoSuchEntry,
@@ -203,8 +211,6 @@ impl From<Error> for c_int {
 #[derive(Clone, Copy, Debug, PartialEq)]
 #[non_exhaustive]
 pub enum ErrorKind {
-    /// Error code was >=0, operation was successful.
-    Success,
     /// Input / output error occurred.
     Io,
     /// File or filesystem was corrupt.
@@ -242,10 +248,9 @@ pub fn error_code_from<T>(result: Result<T>) -> ll::lfs_error {
 }
 
 pub fn result_from<T>(return_value: T, error_code: ll::lfs_error) -> Result<T> {
-    let error = Error::new(error_code);
-    if error.kind() == Some(ErrorKind::Success) {
-        Ok(return_value)
-    } else {
+    if let Some(error) = Error::new(error_code) {
         Err(error)
+    } else {
+        Ok(return_value)
     }
 }