Refactor drive

libcdio-cli/Cargo.toml

@@ -37,10 +37,10 @@ required-features = ["mmc-tool"]
 
 [features]
 default = ["cd-drive", "iso-info", "iso-read", "mmc-tool"]
-cd-drive = []
+cd-drive = ["libcdio-rs/mmc"]
 iso-info = ["libcdio-rs/iso9660", "libcdio-rs/udf", "dep:time"]
 iso-read = ["libcdio-rs/iso9660", "libcdio-rs/udf"]
-mmc-tool = []
+mmc-tool = ["libcdio-rs/mmc"]
 
 [dev-dependencies]
 assert_cmd = { version = "2.2.2", features = ["color"] }

libcdio-cli/src/cd-drive/main.rs

@@ -57,10 +57,10 @@ fn print_drive_info(path: PathBuf) -> Result<()> {
 
 fn print_device_info(drive: &Drive) -> Result<()> {
     println!("Device information:");
-    let info = drive.hardware_info()?;
-    println!("{L1} Vendor   : {}", info.vendor);
-    println!("{L1} Model    : {}", info.model);
-    println!("{L1} Revision : {}", info.revision);
+    let identifiers = drive.hardware_identifiers()?;
+    println!("{L1} Vendor   : {}", identifiers.vendor);
+    println!("{L1} Model    : {}", identifiers.model);
+    println!("{L1} Revision : {}", identifiers.revision);
 
     Ok(())
 }

libcdio-rs/Cargo.toml

@@ -12,6 +12,7 @@ repository.workspace = true
 
 [features]
 iso9660 = ["libcdio-sys/iso9660", "dep:file-mode", "dep:time"]
+mmc = ["dep:winnow"]
 udf = ["libcdio-sys/udf", "dep:file-mode", "dep:time"]
 
 [dependencies]
@@ -24,7 +25,7 @@ num_enum = { version = "0.7.6", features = ["complex-expressions"] }
 thiserror = "2.0.18"
 time = { workspace = true, features = ["local-offset"], optional = true }
 tracing.workspace = true
-winnow = { version = "1.0.3", features = ["binary", "parser", "std"], default-features = false }
+winnow = { version = "1.0.3", features = ["binary", "parser", "std"], default-features = false, optional = true }
 
 [dev-dependencies]
 test-log.workspace = true

libcdio-rs/src/drive.rs

@@ -18,14 +18,14 @@
 //! Routines related to CD/DVD drives.
 
 use std::{
-    ffi::{CStr, CString, NulError, OsString},
+    error::Error,
+    ffi::{CStr, CString, OsString},
     fmt,
     mem::MaybeUninit,
-    path::PathBuf,
+    path::{Path, PathBuf},
 };
 
 use bitflags::bitflags;
-use displaydoc::Display;
 use libcdio_sys::cdio_hwinfo_t;
 use thiserror::Error;
 
@@ -37,8 +37,7 @@ pub struct Drive {
 }
 
 impl Drive {
-    /// Get a list of connected drives.
-    /// The values could be used with [`Self::with_drive()`].
+    /// Returns a list of connected drives.
     pub fn drives() -> Vec<PathBuf> {
         let drive_list =
             unsafe { libcdio_sys::cdio_get_devices(libcdio_sys::driver_id_t_DRIVER_DEVICE) };
@@ -48,68 +47,49 @@ impl Drive {
 
         let mut drives = Vec::new();
         let mut ptr = drive_list;
-        // SAFETY: The device list is NULL terminated, therefore safe to
-        // dereference till NULL is reached
-        while let drive = unsafe { *ptr }
+
+        // SAFETY: Null checked
+        while !ptr.is_null()
+            && let drive = unsafe { *ptr }
             && !drive.is_null()
         {
-            // SAFETY: null check performed; the value represents a path, thus an os string
+            // SAFETY: `drive` represents a system path, making it a valid `OsString`
             drives.push(PathBuf::from(unsafe {
                 OsString::from_encoded_bytes_unchecked(CStr::from_ptr(drive).to_bytes().to_vec())
             }));
             ptr = unsafe { ptr.offset(1) };
         }
 
-        // SAFETY: drive_list has been cloned above, thus safe to free
+        // SAFETY: drive_list has been copied into drives
         unsafe {
             libcdio_sys::cdio_free_device_list(drive_list);
         }
 
         drives
     }
 
-    /// Use a default connected drive.
-    ///
-    /// # Errors
-    /// If there are no drives connected, or the drive could not be opened.
+    /// Opens a default connected drive.
     pub fn new() -> Result<Self, DriveNotFoundError> {
         Cdio::with_device(None)
             .ok_or(DriveNotFoundError)
             .map(|cdio| Self { cdio })
     }
 
-    /// Use the provided drive.
-    ///
-    /// A list of drives can be obtained using [`Self::drives()`].
+    /// Opens drive at given path.
     ///
-    /// # Errors
-    /// - If the device at path could not be opened as a drive
-    /// - If the drive path contains null character
-    pub fn with_drive(drive: PathBuf) -> Result<Self, WithDriveError> {
-        let drive = CString::new(drive.into_os_string().into_encoded_bytes()).map_err(|err| {
-            WithDriveError {
-                drive: os_string_from_bytes_safe(err.clone().into_vec()).into(),
-                source: WithDriveErrorKind::DriveHasNullChar(err),
-            }
-        })?;
-        let cdio = Cdio::with_device(Some(&drive)).ok_or_else(|| WithDriveError {
-            drive: os_string_from_bytes_safe(drive.into_bytes()).into(),
-            source: WithDriveErrorKind::CouldNotOpenAsDrive,
+    /// See [`Self::drives()`] for a list of connected drives.
+    pub fn with_drive(drive: PathBuf) -> Result<Self, DriveOpenError> {
+        let drive = CString::new(drive.into_os_string().into_encoded_bytes())
+            .map_err(|err| DriveOpenError::new(err.clone().into_vec(), err.into()))?;
+        let cdio = Cdio::with_device(Some(&drive)).ok_or_else(|| {
+            DriveOpenError::new(drive.into_bytes(), "cdio_open_am() returned NULL".into())
         })?;
 
-        fn os_string_from_bytes_safe(bytes: Vec<u8>) -> OsString {
-            // SAFETY: the bytes originate from an OsString
-            unsafe { OsString::from_encoded_bytes_unchecked(bytes) }
-        }
-
         Ok(Self { cdio })
     }
 
-    /// Returns hardware information of the drive.
-    ///
-    /// # Errors
-    /// If an underlying operation errored, or if the drive is unavailable.
-    pub fn hardware_info(&self) -> Result<HardwareInfo, DriveOperationError> {
+    /// Returns hardware identifiers of the drive such as Model, Vendor and Revision.
+    pub fn hardware_identifiers(&self) -> Result<HardwareIdentifiers, DriveOperationError> {
         let mut hwinfo: MaybeUninit<cdio_hwinfo_t> = MaybeUninit::uninit();
         let ret = unsafe { libcdio_sys::cdio_get_hwinfo(self.cdio.as_ptr(), hwinfo.as_mut_ptr()) };
         if !ret {
@@ -125,18 +105,15 @@ impl Drive {
             let vendor = CStr::from_ptr(hwinfo.psz_vendor.as_ptr());
             let revision = CStr::from_ptr(hwinfo.psz_revision.as_ptr());
 
-            Ok(HardwareInfo {
+            Ok(HardwareIdentifiers {
                 model: model.to_string_lossy().trim_end().to_string(),
                 vendor: vendor.to_string_lossy().trim_end().to_string(),
                 revision: revision.to_string_lossy().trim_end().to_string(),
             })
         }
     }
 
-    /// Get the drive capabilities.
-    ///
-    /// # Errors
-    /// If the operation errored, or the drive is not available.
+    /// Returns drive capabilities.
     pub fn capabilities(&self) -> Result<DriveCapabilities, DriveOperationError> {
         let mut read = 0;
         let mut write = 0;
@@ -156,40 +133,51 @@ impl Drive {
     }
 }
 
-/// could not find any drives
 #[non_exhaustive]
-#[derive(Debug, Display, Error)]
+#[derive(Debug, Error)]
+#[error("could not find any drives")]
 pub struct DriveNotFoundError;
 
-/// could not perform operation on the drive
-#[non_exhaustive]
-#[derive(Debug, Display, Error)]
-pub struct DriveOperationError;
+#[derive(Debug, Error)]
+#[error(transparent)]
+pub struct DriveOpenError(Box<OpenErrRepr>);
 
-/// error opening drive at `{drive}`
-#[derive(Debug, Display, Error)]
-pub struct WithDriveError {
-    pub drive: PathBuf,
-    pub source: WithDriveErrorKind,
+#[derive(Debug, Error)]
+#[error("could not open drive at `{path}`")]
+struct OpenErrRepr {
+    path: PathBuf,
+    source: Box<dyn Error + Send + Sync>,
 }
-/// Error kind of [`WithDriveError`]
-#[derive(Debug, Display, Error)]
-pub enum WithDriveErrorKind {
-    /// drive path contains null character
-    DriveHasNullChar(NulError),
-    /// could not open device as a drive
-    CouldNotOpenAsDrive,
+
+impl DriveOpenError {
+    /// Returns the system path of the drive.
+    pub fn path(&self) -> &Path {
+        &self.0.path
+    }
+
+    fn new(path_bytes: Vec<u8>, source: Box<dyn Error + Send + Sync>) -> Self {
+        Self(Box::new(OpenErrRepr {
+            // SAFETY: path_bytes originate from a PathBuf
+            path: unsafe { OsString::from_encoded_bytes_unchecked(path_bytes) }.into(),
+            source,
+        }))
+    }
 }
 
-/// Hardware information returned by a cdio driver.
+#[non_exhaustive]
+#[derive(Debug, Error)]
+#[error("could not perform operation on the drive")]
+pub struct DriveOperationError;
+
+/// Hardware identifiers such as model, vendor and revision.
 #[derive(Clone, Debug, PartialEq, Eq)]
-pub struct HardwareInfo {
+pub struct HardwareIdentifiers {
     pub model: String,
     pub vendor: String,
     pub revision: String,
 }
 
-/// Drive capabilities
+/// Drive capabilities.
 #[derive(Clone, Copy, Debug)]
 pub struct DriveCapabilities {
     pub read: ReadCapabilities,
@@ -198,6 +186,7 @@ pub struct DriveCapabilities {
 }
 // the C enum discriminants are explicit, positive and fit a u32, making these casts safe
 bitflags! {
+    /// Miscellaneous capabilities of the drive.
     #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
     pub struct MiscCapabilities: u32 {
         /// Can close tray
@@ -220,7 +209,7 @@ bitflags! {
 }
 // the C enum discriminants are explicit, positive and fit a u32, making these casts safe
 bitflags! {
-    /// Read capabilities of the drive
+    /// Read capabilities of the drive.
     #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
     pub struct ReadCapabilities: u32 {
         /// Can play audio
@@ -259,7 +248,7 @@ bitflags! {
 }
 // the C enum discriminants are explicit, positive and fit a u32, making these casts safe
 bitflags! {
-    /// Write capabilities of the drive
+    /// Write capabilities of the drive.
     #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
     pub struct WriteCapabilities: u32 {
         /// Can write CD-R
@@ -355,8 +344,8 @@ mod tests {
 
     #[test]
     #[ignore = "requires a disc drive"]
-    fn hardware_info() {
-        Drive::new().unwrap().hardware_info().unwrap();
+    fn hardware_identifiers() {
+        Drive::new().unwrap().hardware_identifiers().unwrap();
     }
 
     #[test]

libcdio-rs/src/lib.rs

@@ -24,13 +24,15 @@ pub mod drive;
 pub mod iso9660;
 
 mod logging;
+
+#[cfg(feature = "mmc")]
 pub mod mmc;
 
 #[cfg(feature = "udf")]
 pub mod udf;
 
 #[doc(inline)]
-pub use crate::{drive::Drive, mmc::Mmc};
+pub use crate::drive::Drive;
 
 #[cfg(feature = "iso9660")]
 #[doc(inline)]
@@ -40,5 +42,9 @@ pub use crate::iso9660::Iso;
 #[doc(inline)]
 pub use crate::udf::Udf;
 
+#[cfg(feature = "mmc")]
+#[doc(inline)]
+pub use mmc::Mmc;
+
 #[cfg(any(feature = "iso9660", feature = "udf"))]
 pub use {file_mode, time};