From 2d1fb106305804b03a946b82892a02c3a90c31ea Mon Sep 17 00:00:00 2001 From: Shiva Kiran Koninty Date: Tue, 18 Aug 2026 13:56:33 +0530 Subject: [PATCH 1/4] lib/drive: Replace `WithDriveError` with simpler `DriveOpenError` --- libcdio-rs/src/drive.rs | 74 ++++++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 38 deletions(-) diff --git a/libcdio-rs/src/drive.rs b/libcdio-rs/src/drive.rs index 06dce7f..cf62ed2 100644 --- a/libcdio-rs/src/drive.rs +++ b/libcdio-rs/src/drive.rs @@ -18,10 +18,11 @@ //! 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; @@ -78,30 +79,16 @@ impl Drive { .map(|cdio| Self { cdio }) } - /// Use the provided drive. + /// Opens drive at given path. /// - /// A list of drives can be obtained using [`Self::drives()`]. - /// - /// # 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 { - 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 { + 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) -> OsString { - // SAFETY: the bytes originate from an OsString - unsafe { OsString::from_encoded_bytes_unchecked(bytes) } - } - Ok(Self { cdio }) } @@ -161,26 +148,37 @@ impl Drive { #[derive(Debug, Display, Error)] pub struct DriveNotFoundError; +#[derive(Debug, Error)] +#[error(transparent)] +pub struct DriveOpenError(Box); + +#[derive(Debug, Error)] +#[error("could not open drive at `{path}`")] +struct OpenErrRepr { + path: PathBuf, + source: Box, +} + +impl DriveOpenError { + /// Returns the system path of the drive. + pub fn path(&self) -> &Path { + &self.0.path + } + + fn new(path_bytes: Vec, source: Box) -> Self { + Self(Box::new(OpenErrRepr { + // SAFETY: path_bytes originate from a PathBuf + path: unsafe { OsString::from_encoded_bytes_unchecked(path_bytes) }.into(), + source, + })) + } +} + /// could not perform operation on the drive #[non_exhaustive] #[derive(Debug, Display, Error)] pub struct DriveOperationError; -/// error opening drive at `{drive}` -#[derive(Debug, Display, Error)] -pub struct WithDriveError { - pub drive: PathBuf, - pub source: WithDriveErrorKind, -} -/// 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, -} - /// Hardware information returned by a cdio driver. #[derive(Clone, Debug, PartialEq, Eq)] pub struct HardwareInfo { From 4f62fd2552b28de066d803e9845ddca236dab447 Mon Sep 17 00:00:00 2001 From: Shiva Kiran Koninty Date: Tue, 18 Aug 2026 15:21:19 +0530 Subject: [PATCH 2/4] lib/drive: Revise comments --- libcdio-rs/src/drive.rs | 45 +++++++++++++++++------------------------ 1 file changed, 18 insertions(+), 27 deletions(-) diff --git a/libcdio-rs/src/drive.rs b/libcdio-rs/src/drive.rs index cf62ed2..ee40157 100644 --- a/libcdio-rs/src/drive.rs +++ b/libcdio-rs/src/drive.rs @@ -26,7 +26,6 @@ use std::{ }; use bitflags::bitflags; -use displaydoc::Display; use libcdio_sys::cdio_hwinfo_t; use thiserror::Error; @@ -38,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 { let drive_list = unsafe { libcdio_sys::cdio_get_devices(libcdio_sys::driver_id_t_DRIVER_DEVICE) }; @@ -49,19 +47,20 @@ 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); } @@ -69,10 +68,7 @@ impl Drive { 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 { Cdio::with_device(None) .ok_or(DriveNotFoundError) @@ -93,9 +89,6 @@ impl Drive { } /// Returns hardware information of the drive. - /// - /// # Errors - /// If an underlying operation errored, or if the drive is unavailable. pub fn hardware_info(&self) -> Result { let mut hwinfo: MaybeUninit = MaybeUninit::uninit(); let ret = unsafe { libcdio_sys::cdio_get_hwinfo(self.cdio.as_ptr(), hwinfo.as_mut_ptr()) }; @@ -120,10 +113,7 @@ impl Drive { } } - /// Get the drive capabilities. - /// - /// # Errors - /// If the operation errored, or the drive is not available. + /// Returns drive capabilities. pub fn capabilities(&self) -> Result { let mut read = 0; let mut write = 0; @@ -143,9 +133,9 @@ 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; #[derive(Debug, Error)] @@ -174,12 +164,12 @@ impl DriveOpenError { } } -/// could not perform operation on the drive #[non_exhaustive] -#[derive(Debug, Display, Error)] +#[derive(Debug, Error)] +#[error("could not perform operation on the drive")] pub struct DriveOperationError; -/// Hardware information returned by a cdio driver. +/// Hardware identifiers such as model, vendor and revision. #[derive(Clone, Debug, PartialEq, Eq)] pub struct HardwareInfo { pub model: String, @@ -187,7 +177,7 @@ pub struct HardwareInfo { pub revision: String, } -/// Drive capabilities +/// Drive capabilities. #[derive(Clone, Copy, Debug)] pub struct DriveCapabilities { pub read: ReadCapabilities, @@ -196,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 @@ -218,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 @@ -257,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 From 24ad96d29d93462902979e99cd6b0bd0262406fa Mon Sep 17 00:00:00 2001 From: Shiva Kiran Koninty Date: Fri, 21 Aug 2026 13:31:46 +0530 Subject: [PATCH 3/4] lib/drive: Rename `hardware_info()` to `hardware_identifiers()` --- libcdio-cli/src/cd-drive/main.rs | 8 ++++---- libcdio-rs/src/drive.rs | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/libcdio-cli/src/cd-drive/main.rs b/libcdio-cli/src/cd-drive/main.rs index c55a901..2c032e8 100644 --- a/libcdio-cli/src/cd-drive/main.rs +++ b/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(()) } diff --git a/libcdio-rs/src/drive.rs b/libcdio-rs/src/drive.rs index ee40157..90e4a29 100644 --- a/libcdio-rs/src/drive.rs +++ b/libcdio-rs/src/drive.rs @@ -88,8 +88,8 @@ impl Drive { Ok(Self { cdio }) } - /// Returns hardware information of the drive. - pub fn hardware_info(&self) -> Result { + /// Returns hardware identifiers of the drive such as Model, Vendor and Revision. + pub fn hardware_identifiers(&self) -> Result { let mut hwinfo: MaybeUninit = MaybeUninit::uninit(); let ret = unsafe { libcdio_sys::cdio_get_hwinfo(self.cdio.as_ptr(), hwinfo.as_mut_ptr()) }; if !ret { @@ -105,7 +105,7 @@ 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(), @@ -171,7 +171,7 @@ 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, @@ -344,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] From d622d32722801689e65b77520cd2fda4c4794a55 Mon Sep 17 00:00:00 2001 From: Shiva Kiran Koninty Date: Fri, 21 Aug 2026 13:47:50 +0530 Subject: [PATCH 4/4] lib: Gate the mmc module behind a feature --- libcdio-cli/Cargo.toml | 4 ++-- libcdio-rs/Cargo.toml | 3 ++- libcdio-rs/src/lib.rs | 8 +++++++- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/libcdio-cli/Cargo.toml b/libcdio-cli/Cargo.toml index 9cdbcab..f0d4b2d 100644 --- a/libcdio-cli/Cargo.toml +++ b/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"] } diff --git a/libcdio-rs/Cargo.toml b/libcdio-rs/Cargo.toml index ab1591a..abfad0d 100644 --- a/libcdio-rs/Cargo.toml +++ b/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 diff --git a/libcdio-rs/src/lib.rs b/libcdio-rs/src/lib.rs index 90927d2..9eba35e 100644 --- a/libcdio-rs/src/lib.rs +++ b/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};