Use Newtypes for NotifyState

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "storefd"
-version = "0.1.1"
+version = "0.2.0-dev"
 edition = "2021"
 license = "MIT"
 authors = ["Mathieu Trossevin <mtrossevin@evolix.fr>"]

src/notify/error.rs

@@ -41,24 +41,14 @@ impl Error for NewNotifierError {
 
 #[derive(Debug)]
 pub enum NotifyError {
-    SanityCheck(SanityCheckError),
     SendMsg(Errno),
     PushAncillaryMessage,
     PartialSend,
 }
 
-impl From<SanityCheckError> for NotifyError {
-    fn from(value: SanityCheckError) -> Self {
-        Self::SanityCheck(value)
-    }
-}
-
 impl Display for NotifyError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            NotifyError::SanityCheck(error) => {
-                write!(f, "NotifyState sanity check failed: {error}")
-            }
             NotifyError::SendMsg(error) => write!(f, "couldn't send the message: {error}"),
             NotifyError::PushAncillaryMessage => {
                 f.write_str("couldn't push necessary ancillary message for fd passing")
@@ -71,36 +61,12 @@ impl Display for NotifyError {
 impl Error for NotifyError {
     fn source(&self) -> Option<&(dyn Error + 'static)> {
         match self {
-            NotifyError::SanityCheck(error) => Some(error),
             NotifyError::SendMsg(error) => Some(error),
             NotifyError::PushAncillaryMessage | NotifyError::PartialSend => None,
         }
     }
 }
 
-#[derive(Debug)]
-pub enum SanityCheckError {
-    InvalidFdName(FdNameError),
-}
-
-impl Display for SanityCheckError {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            SanityCheckError::InvalidFdName(error) => {
-                write!(f, "the value of FDNAME was invalid : {error}")
-            }
-        }
-    }
-}
-
-impl Error for SanityCheckError {
-    fn source(&self) -> Option<&(dyn Error + 'static)> {
-        match self {
-            SanityCheckError::InvalidFdName(error) => Some(error),
-        }
-    }
-}
-
 #[derive(Debug)]
 pub enum FdNameError {
     TooLong { length: usize, name: String },
@@ -119,3 +85,20 @@ impl Display for FdNameError {
 }
 
 impl Error for FdNameError {}
+
+#[derive(Debug)]
+pub enum StatusLineError {
+    TooManyLines,
+}
+
+impl Display for StatusLineError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::TooManyLines => {
+                f.write_str("The provided status line contains more than one line.")
+            }
+        }
+    }
+}
+
+impl Error for StatusLineError {}

src/notify/mod.rs

@@ -22,9 +22,10 @@ use std::os::unix::net::UnixDatagram;
 
 use rustix::fd::{AsFd, BorrowedFd};
 
-use self::error::{FdNameError, NewNotifierError, NotifyError, SanityCheckError};
+use self::error::{NewNotifierError, NotifyError};
 
 pub mod error;
+pub mod types;
 
 /// A wrapper around the socket specified by `$NOTIFY_SOCKET`
 #[derive(Debug)]
@@ -104,8 +105,6 @@ impl Notifier {
             tracing::info_span!("notify_with_fds", f_self = ?self, f_state = ?state, f_fds = ?fds);
         let _enter = span.enter();
 
-        state.iter().try_for_each(NotifyState::sanity_check)?;
-
         let msg = state
             .iter()
             .fold(String::new(), |acc, state| format!("{acc}{state}\n"))
@@ -201,11 +200,11 @@ pub fn is_watchdog_enabled(unset_env: bool) -> Option<std::time::Duration> {
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum NotifyState<'a> {
     /// D-Bus error-style error code.
-    BusError(&'a str),
+    BusError(types::BusError<'a>),
     /// errno-style error code.
     Errno(u8),
     /// A name for the submitted file descriptors.
-    FdName(&'a str),
+    FdName(types::FdName<'a>),
     /// Stores additional file descriptors in the service manager. Use [`Notifier::notify_with_fds()`] with this.
     FdStore,
     /// Remove stored file descriptors. Must be used together with [`NotifyState::FdName`].
@@ -223,7 +222,7 @@ pub enum NotifyState<'a> {
     /// Service is reloading.
     Reloading,
     /// Custom status change.
-    Status(&'a str),
+    Status(types::StatusLine<'a>),
     /// Service is beginning to shutdown.
     Stopping,
     /// Tell the service manager to update the watchdog timestamp.
@@ -232,18 +231,18 @@ pub enum NotifyState<'a> {
     WatchdogTrigger,
     /// Reset watchdog timeout value during runtime.
     /// The value is in milliseconds.
-    WatchdogUsec(u64),
+    WatchdogUsec(types::Milliseconds),
     /// Tells the service manager to extend the startup, runtime or shutdown service timeout corresponding the current state.
     /// The value is in milliseconds.
-    ExtendTimeoutUsec(u64),
+    ExtendTimeoutUsec(types::Milliseconds),
 }
 
 impl<'a> Display for NotifyState<'a> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match *self {
-            NotifyState::BusError(s) => write!(f, "BUSERROR={s}"),
+            NotifyState::BusError(types::BusError(s)) => write!(f, "BUSERROR={s}"),
             NotifyState::Errno(e) => write!(f, "ERRNO={e}"),
-            NotifyState::FdName(name) => write!(f, "FDNAME={name}"),
+            NotifyState::FdName(types::FdName(name)) => write!(f, "FDNAME={name}"),
             NotifyState::FdStore => f.write_str("FDSTORE=1"),
             NotifyState::FdStoreRemove => f.write_str("FDSTOREREMOVE=1"),
             NotifyState::FdpollDisable => f.write_str("FDPOLL=0"),
@@ -251,48 +250,16 @@ impl<'a> Display for NotifyState<'a> {
             NotifyState::Other(message) => Display::fmt(message, f),
             NotifyState::Ready => f.write_str("READY=1"),
             NotifyState::Reloading => f.write_str("RELOADING=1"),
-            NotifyState::Status(status) => write!(f, "STATUS={status}"),
+            NotifyState::Status(types::StatusLine(status)) => write!(f, "STATUS={status}"),
             NotifyState::Stopping => f.write_str("STOPPING=1"),
             NotifyState::Watchdog => f.write_str("WATCHDOG=1"),
             NotifyState::WatchdogTrigger => f.write_str("WATCHDOG=trigger"),
-            NotifyState::WatchdogUsec(milliseconds) => write!(f, "WATCHDOG_USEC={milliseconds}"),
-            NotifyState::ExtendTimeoutUsec(milliseconds) => {
+            NotifyState::WatchdogUsec(types::Milliseconds(milliseconds)) => {
+                write!(f, "WATCHDOG_USEC={milliseconds}")
+            }
+            NotifyState::ExtendTimeoutUsec(types::Milliseconds(milliseconds)) => {
                 write!(f, "EXTEND_TIMEOUT_USEC={milliseconds}")
             }
         }
     }
 }
-
-impl<'a> NotifyState<'a> {
-    fn sanity_check(&self) -> Result<(), SanityCheckError> {
-        tracing::debug!("Checking that NotifyState {self:?} respect SystemD's expectations.");
-        match self {
-            NotifyState::FdName(name) => {
-                validate_fd_name(name).map_err(SanityCheckError::InvalidFdName)
-            }
-            _ => Ok(()),
-        }
-    }
-}
-
-fn validate_fd_name(name: &str) -> Result<(), FdNameError> {
-    if name.len() > 255 {
-        return Err(FdNameError::TooLong {
-            length: name.len(),
-            name: name.into(),
-        });
-    }
-
-    for c in name.chars() {
-        if !c.is_ascii() || c.is_ascii_control() {
-            return Err(FdNameError::NotAsciiNonControl {
-                disallowed_char: c,
-                name: name.into(),
-            });
-        }
-        if c == ':' {
-            return Err(FdNameError::ContainColon(name.into()));
-        }
-    }
-    Ok(())
-}

src/notify/types.rs

@@ -0,0 +1,104 @@
+//! Newtypes used by [`NotifyState`](super::NotifyState)
+
+use super::error;
+
+/// Allowed File descriptor name.
+///
+/// A name is allowed when it :
+///
+/// * Has less than 255 characters.
+/// * Is ASCII.
+/// * Doesn't contains control characters.
+/// * Doesn't contains a colon (`:`).
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct FdName<'a>(pub(super) &'a str);
+
+impl<'a> TryFrom<&'a str> for FdName<'a> {
+    type Error = error::FdNameError;
+
+    fn try_from(name: &'a str) -> Result<Self, Self::Error> {
+        if name.len() > 255 {
+            return Err(error::FdNameError::TooLong {
+                length: name.len(),
+                name: name.into(),
+            });
+        }
+
+        for c in name.chars() {
+            if !c.is_ascii() || c.is_ascii_control() {
+                return Err(error::FdNameError::NotAsciiNonControl {
+                    disallowed_char: c,
+                    name: name.into(),
+                });
+            }
+            if c == ':' {
+                return Err(error::FdNameError::ContainColon(name.into()));
+            }
+        }
+
+        Ok(Self(name))
+    }
+}
+
+impl AsRef<str> for FdName<'_> {
+    fn as_ref(&self) -> &str {
+        self.0
+    }
+}
+
+/// A status line for [`NotifyState::Status`](super::NotifyState::Status).
+///
+/// As the name explains it needs to be a single line.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct StatusLine<'a>(pub(super) &'a str);
+
+impl<'a> TryFrom<&'a str> for StatusLine<'a> {
+    type Error = error::StatusLineError;
+
+    fn try_from(value: &'a str) -> Result<Self, Self::Error> {
+        if value.lines().nth(1).is_some() {
+            return Err(Self::Error::TooManyLines);
+        }
+
+        Ok(Self(value))
+    }
+}
+
+impl AsRef<str> for StatusLine<'_> {
+    fn as_ref(&self) -> &str {
+        self.0
+    }
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct Milliseconds(pub(super) u64);
+
+impl From<u64> for Milliseconds {
+    fn from(value: u64) -> Self {
+        Self(value)
+    }
+}
+
+impl AsRef<u64> for Milliseconds {
+    fn as_ref(&self) -> &u64 {
+        &self.0
+    }
+}
+
+/// A D-Bus error-style error code.
+///
+/// Right now it doesn't impose any additional constraint on [`str`]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+pub struct BusError<'a>(pub(super) &'a str);
+
+impl<'a> From<&'a str> for BusError<'a> {
+    fn from(value: &'a str) -> Self {
+        Self(value)
+    }
+}
+
+impl AsRef<str> for BusError<'_> {
+    fn as_ref(&self) -> &str {
+        self.0
+    }
+}