From 7f394075abd1346f579d3b73e7c21b1a4215f8b1 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 5 May 2026 13:36:54 +0000 Subject: [PATCH 1/7] Fix macOS build and initialize SCSI task device - Added missing IOKit SCSI headers to shim_common.h. - Implemented SCSI Task Device initialization in device_service.c by traversing the IORegistry parent chain. - Corrected the SCSI transfer direction in data_reader.c. - These changes restore the ability to perform direct SCSI commands for data sector reading on macOS. Co-authored-by: danifunker <1643018+danifunker@users.noreply.github.com> --- build.rs | 2 + src/data_reader.rs | 320 +++++++++++++++++++++++++++++++++++++++ src/lib.rs | 17 +++ src/linux.rs | 10 ++ src/mac/data_reader.c | 146 ++++++++++++++++++ src/mac/device_service.c | 81 +++++++++- src/mac/shim_common.h | 4 + src/macos.rs | 41 +++++ src/windows.rs | 11 ++ 9 files changed, 631 insertions(+), 1 deletion(-) create mode 100644 src/data_reader.rs create mode 100644 src/mac/data_reader.c diff --git a/build.rs b/build.rs index eabe876..f930ee6 100644 --- a/build.rs +++ b/build.rs @@ -6,6 +6,7 @@ fn main() { println!("cargo:rerun-if-changed=src/mac/list_drives.c"); println!("cargo:rerun-if-changed=src/mac/toc_reader.c"); println!("cargo:rerun-if-changed=src/mac/audio_reader.c"); + println!("cargo:rerun-if-changed=src/mac/data_reader.c"); println!("cargo:rustc-link-lib=framework=IOKit"); println!("cargo:rustc-link-lib=framework=CoreFoundation"); @@ -14,6 +15,7 @@ fn main() { .file("src/mac/list_drives.c") .file("src/mac/toc_reader.c") .file("src/mac/audio_reader.c") + .file("src/mac/data_reader.c") .include("src/mac") // force C compilation .flag("-x") diff --git a/src/data_reader.rs b/src/data_reader.rs new file mode 100644 index 0000000..c5ac86e --- /dev/null +++ b/src/data_reader.rs @@ -0,0 +1,320 @@ +/// Sector read mode for the READ CD (0xBE) command. +/// +/// Controls CDB byte 1 (Expected Sector Type) and byte 9 (Main Channel Selection) +/// to read different sector formats from the same READ CD command. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SectorReadMode { + /// Audio: 2352 bytes/sector, raw PCM. + /// CDB byte 1 = 0x00 (any type), byte 9 = 0x10 (user data). + Audio, + /// Data cooked: 2048 bytes/sector, user data only (no sync/header/EDC/ECC). + /// CDB byte 1 = 0x04 (Mode 1), byte 9 = 0x10 (user data). + DataCooked, + /// Data raw: 2352 bytes/sector with sync + header + user data + EDC/ECC. + /// CDB byte 1 = 0x04 (Mode 1), byte 9 = 0xF8 (sync + header + user data + EDC/ECC). + DataRaw, +} + +impl SectorReadMode { + /// Bytes per sector for this read mode. + pub fn sector_size(&self) -> usize { + match self { + SectorReadMode::Audio => 2352, + SectorReadMode::DataCooked => 2048, + SectorReadMode::DataRaw => 2352, + } + } + + /// CDB byte 1: Expected Sector Type field (bits 5-2). + pub fn cdb_byte1(&self) -> u8 { + match self { + SectorReadMode::Audio => 0x00, + SectorReadMode::DataCooked => 0x04, + SectorReadMode::DataRaw => 0x04, + } + } + + /// CDB byte 9: Main Channel Selection bits. + pub fn cdb_byte9(&self) -> u8 { + match self { + SectorReadMode::Audio => 0x10, + SectorReadMode::DataCooked => 0x10, + SectorReadMode::DataRaw => 0xF8, + } + } +} + +/// Read sectors from the disc in the specified mode with retry logic. +/// +/// This parallels the existing `read_sectors_with_retry` but allows choosing +/// between audio, cooked data, and raw data sector formats. +pub fn read_data_sectors( + lba: u32, + sectors: u32, + mode: SectorReadMode, + cfg: &crate::RetryConfig, +) -> Result, crate::CdReaderError> { + let sector_size = mode.sector_size(); + let max_sectors_per_xfer: u32 = match sector_size { + 2048 => 32, // 32 * 2048 = 65536 bytes + _ => 27, // 27 * 2352 = 63504 bytes + }; + + let total_bytes = (sectors as usize) * sector_size; + let mut out = Vec::::with_capacity(total_bytes); + + let mut remaining = sectors; + let mut cur_lba = lba; + let attempts_total = cfg.max_attempts.max(1); + + while remaining > 0 { + let mut chunk_sectors = remaining.min(max_sectors_per_xfer); + let min_chunk = cfg.min_sectors_per_read.max(1); + let mut backoff_ms = cfg.initial_backoff_ms; + let mut last_err: Option = None; + + for attempt in 1..=attempts_total { + match read_data_chunk(cur_lba, chunk_sectors, &mode) { + Ok(chunk) => { + out.extend_from_slice(&chunk); + cur_lba += chunk_sectors; + remaining -= chunk_sectors; + last_err = None; + break; + } + Err(err) => { + last_err = Some(err); + if attempt == attempts_total { + break; + } + if cfg.reduce_chunk_on_retry && chunk_sectors > min_chunk { + chunk_sectors = next_chunk_size(chunk_sectors, min_chunk); + } + if backoff_ms > 0 { + std::thread::sleep(std::time::Duration::from_millis(backoff_ms)); + } + if cfg.max_backoff_ms > 0 { + backoff_ms = (backoff_ms.saturating_mul(2)).min(cfg.max_backoff_ms); + } + } + } + } + + if let Some(err) = last_err { + return Err(err); + } + } + + Ok(out) +} + +fn next_chunk_size(current: u32, min_chunk: u32) -> u32 { + if current > 8 { + 8.max(min_chunk) + } else { + min_chunk + } +} + +// ── Platform-specific read_data_chunk implementations ── + +#[cfg(target_os = "linux")] +fn read_data_chunk( + lba: u32, + this_sectors: u32, + mode: &SectorReadMode, +) -> Result, crate::CdReaderError> { + use crate::{CdReaderError, ScsiError, ScsiOp}; + use libc::{c_uchar, c_void}; + + let sector_size = mode.sector_size(); + let mut chunk = vec![0u8; (this_sectors as usize) * sector_size]; + let mut sense = vec![0u8; 64]; + + let mut cdb = [0u8; 12]; + cdb[0] = 0xBE; // READ CD + cdb[1] = mode.cdb_byte1(); + cdb[2] = ((lba >> 24) & 0xFF) as u8; + cdb[3] = ((lba >> 16) & 0xFF) as u8; + cdb[4] = ((lba >> 8) & 0xFF) as u8; + cdb[5] = (lba & 0xFF) as u8; + cdb[6] = ((this_sectors >> 16) & 0xFF) as u8; + cdb[7] = ((this_sectors >> 8) & 0xFF) as u8; + cdb[8] = (this_sectors & 0xFF) as u8; + cdb[9] = mode.cdb_byte9(); + + let fd = crate::linux::get_drive_fd()?; + + // Build SgIoHeader inline (same layout as linux.rs) + #[repr(C)] + struct SgIoHeader { + interface_id: i32, + dxfer_direction: i32, + cmd_len: u8, + mx_sb_len: u8, + iovec_count: u16, + dxfer_len: u32, + dxferp: *mut c_void, + cmdp: *mut c_uchar, + sbp: *mut c_uchar, + timeout: u32, + flags: u32, + pack_id: i32, + usr_ptr: *mut c_void, + status: u8, + masked_status: u8, + msg_status: u8, + sb_len_wr: u8, + host_status: u16, + driver_status: u16, + resid: i32, + duration: u32, + info: u32, + } + + const SG_DXFER_FROM_DEV: i32 = -3; + const SG_INFO_CHECK: u32 = 0x1; + const SG_IO: u64 = 0x2285; + + let mut hdr = SgIoHeader { + interface_id: b'S' as i32, + dxfer_direction: SG_DXFER_FROM_DEV, + cmd_len: cdb.len() as u8, + mx_sb_len: sense.len() as u8, + iovec_count: 0, + dxfer_len: chunk.len() as u32, + dxferp: chunk.as_mut_ptr() as *mut c_void, + cmdp: cdb.as_mut_ptr(), + sbp: sense.as_mut_ptr(), + timeout: 30_000, + flags: 0, + pack_id: 0, + usr_ptr: std::ptr::null_mut(), + status: 0, + masked_status: 0, + msg_status: 0, + sb_len_wr: 0, + host_status: 0, + driver_status: 0, + resid: 0, + duration: 0, + info: 0, + }; + + let ret = unsafe { libc::ioctl(fd, SG_IO, &mut hdr as *mut _) }; + if ret < 0 { + return Err(CdReaderError::Io(std::io::Error::last_os_error())); + } + + if hdr.info & SG_INFO_CHECK != 0 || hdr.status != 0 { + let (sense_key, asc, ascq) = parse_sense(&sense, hdr.sb_len_wr); + return Err(CdReaderError::Scsi(ScsiError { + op: ScsiOp::ReadCd, + lba: Some(lba), + sectors: Some(this_sectors), + scsi_status: hdr.status, + sense_key, + asc, + ascq, + })); + } + + if hdr.resid > 0 { + let got = (chunk.len() as i32 - hdr.resid).max(0) as usize; + chunk.truncate(got); + } + + Ok(chunk) +} + +#[cfg(target_os = "windows")] +fn read_data_chunk( + lba: u32, + this_sectors: u32, + mode: &SectorReadMode, +) -> Result, crate::CdReaderError> { + use crate::windows::SptdWithSense; + use crate::{CdReaderError, ScsiError, ScsiOp}; + use windows_sys::Win32::Storage::IscsiDisc::{ + IOCTL_SCSI_PASS_THROUGH_DIRECT, SCSI_IOCTL_DATA_IN, SCSI_PASS_THROUGH_DIRECT, + }; + use windows_sys::Win32::System::IO::DeviceIoControl; + + let sector_size = mode.sector_size(); + let mut chunk = vec![0u8; (this_sectors as usize) * sector_size]; + + let mut wrapper: SptdWithSense = unsafe { std::mem::zeroed() }; + let sptd = &mut wrapper.sptd; + + sptd.Length = size_of::() as u16; + sptd.CdbLength = 12; + sptd.DataIn = SCSI_IOCTL_DATA_IN as u8; + sptd.TimeOutValue = 30; + sptd.DataTransferLength = chunk.len() as u32; + sptd.DataBuffer = chunk.as_mut_ptr() as *mut _; + sptd.SenseInfoLength = wrapper.sense.len() as u8; + sptd.SenseInfoOffset = size_of::() as u32; + + let cdb = &mut sptd.Cdb; + cdb.fill(0); + cdb[0] = 0xBE; + cdb[1] = mode.cdb_byte1(); + cdb[2] = ((lba >> 24) & 0xFF) as u8; + cdb[3] = ((lba >> 16) & 0xFF) as u8; + cdb[4] = ((lba >> 8) & 0xFF) as u8; + cdb[5] = (lba & 0xFF) as u8; + cdb[6] = ((this_sectors >> 16) & 0xFF) as u8; + cdb[7] = ((this_sectors >> 8) & 0xFF) as u8; + cdb[8] = (this_sectors & 0xFF) as u8; + cdb[9] = mode.cdb_byte9(); + + let handle = crate::windows::get_drive_handle()?; + let mut bytes = 0u32; + let ok = unsafe { + DeviceIoControl( + handle, + IOCTL_SCSI_PASS_THROUGH_DIRECT, + &mut wrapper as *mut _ as *mut _, + size_of::() as u32, + &mut wrapper as *mut _ as *mut _, + size_of::() as u32, + &mut bytes as *mut u32, + std::ptr::null_mut(), + ) + }; + + if ok == 0 { + return Err(CdReaderError::Io(std::io::Error::last_os_error())); + } + if wrapper.sptd.ScsiStatus != 0 { + let (sense_key, asc, ascq) = parse_sense(&wrapper.sense, wrapper.sptd.SenseInfoLength); + return Err(CdReaderError::Scsi(ScsiError { + op: ScsiOp::ReadCd, + lba: Some(lba), + sectors: Some(this_sectors), + scsi_status: wrapper.sptd.ScsiStatus, + sense_key, + asc, + ascq, + })); + } + + Ok(chunk) +} + +#[cfg(target_os = "macos")] +fn read_data_chunk( + lba: u32, + this_sectors: u32, + mode: &SectorReadMode, +) -> Result, crate::CdReaderError> { + crate::macos::read_data_chunk(lba, this_sectors, mode) +} + +#[cfg(any(target_os = "linux", target_os = "windows"))] +fn parse_sense(sense: &[u8], sb_len_wr: u8) -> (Option, Option, Option) { + if sb_len_wr == 0 || sense.len() < 14 { + return (None, None, None); + } + (Some(sense[2] & 0x0F), Some(sense[12]), Some(sense[13])) +} diff --git a/src/lib.rs b/src/lib.rs index 3f9ddd3..108f41d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -150,11 +150,13 @@ mod macos; #[cfg(target_os = "windows")] mod windows; +pub mod data_reader; mod discovery; mod errors; mod retry; mod stream; mod utils; +pub use data_reader::SectorReadMode; pub use discovery::DriveInfo; pub use errors::{CdReaderError, ScsiError, ScsiOp}; pub use retry::RetryConfig; @@ -321,6 +323,21 @@ impl CdReader { } } + /// Read sectors in a specific mode (audio, data cooked, or data raw). + /// + /// This uses the READ CD (0xBE) SCSI command with configurable sector type + /// and main channel flags, supporting audio, cooked data (2048 B/sector), + /// and raw data (2352 B/sector) reads. + pub fn read_data_sectors( + &self, + lba: u32, + count: u32, + mode: SectorReadMode, + cfg: &RetryConfig, + ) -> Result, CdReaderError> { + data_reader::read_data_sectors(lba, count, mode, cfg) + } + pub(crate) fn read_sectors_with_retry( &self, start_lba: u32, diff --git a/src/linux.rs b/src/linux.rs index 0ced759..0769135 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -85,6 +85,16 @@ pub fn open_drive(path: &str) -> std::io::Result<()> { Ok(()) } +#[allow(static_mut_refs)] +pub(crate) fn get_drive_fd() -> Result { + use std::os::fd::AsRawFd; + unsafe { + DRIVE_HANDLE.as_ref().map(|f| f.as_raw_fd()).ok_or_else(|| { + CdReaderError::Io(Error::new(std::io::ErrorKind::NotFound, "Drive not opened")) + }) + } +} + #[allow(static_mut_refs)] pub fn close_drive() { unsafe { diff --git a/src/mac/data_reader.c b/src/mac/data_reader.c new file mode 100644 index 0000000..dc9ec77 --- /dev/null +++ b/src/mac/data_reader.c @@ -0,0 +1,146 @@ +#include "shim_common.h" + +static uint8_t task_status_to_scsi_status(SCSITaskStatus status) { + switch (status) { + case kSCSITaskStatus_GOOD: return 0x00; + case kSCSITaskStatus_CHECK_CONDITION: return 0x02; + case kSCSITaskStatus_BUSY: return 0x08; + case kSCSITaskStatus_RESERVATION_CONFLICT: return 0x18; + case kSCSITaskStatus_TASK_SET_FULL: return 0x28; + case kSCSITaskStatus_ACA_ACTIVE: return 0x30; + default: return 0xFF; + } +} + +static void fill_data_scsi_error(CdScsiError *outErr, kern_return_t ex, SCSITaskStatus status, SCSI_Sense_Data *sense) { + if (!outErr) return; + + outErr->has_scsi_error = 1; + outErr->exec_error = (uint32_t)ex; + outErr->task_status = (uint32_t)status; + outErr->scsi_status = task_status_to_scsi_status(status); + + const uint8_t *sense_bytes = (const uint8_t *)sense; + bool has_sense = false; + for (size_t i = 0; i < sizeof(SCSI_Sense_Data); i++) { + if (sense_bytes[i] != 0) { + has_sense = true; + break; + } + } + + outErr->has_sense = has_sense ? 1 : 0; + if (has_sense && sizeof(SCSI_Sense_Data) >= 14) { + outErr->sense_key = sense_bytes[2] & 0x0F; + outErr->asc = sense_bytes[12]; + outErr->ascq = sense_bytes[13]; + } +} + +bool read_cd_data(uint32_t lba, uint32_t sectors, + uint8_t cdb_byte1, uint8_t cdb_byte9, uint32_t sector_size, + uint8_t **outBuf, uint32_t *outLen, CdScsiError *outErr) { + *outBuf = NULL; + *outLen = 0; + if (outErr) { + memset(outErr, 0, sizeof(CdScsiError)); + } + + SCSITaskDeviceInterface **dev = globalDev; + if (!dev) { + fprintf(stderr, "[READ_DATA] Device session is not open\n"); + goto fail; + } + + if (sectors == 0) { + fprintf(stderr, "[READ_DATA] sectors == 0\n"); + goto fail; + } + + uint64_t totalBytes64 = (uint64_t)sector_size * (uint64_t)sectors; + if (totalBytes64 > UINT32_MAX) { + fprintf(stderr, "[READ_DATA] requested size too large\n"); + goto fail; + } + uint32_t totalBytes = (uint32_t)totalBytes64; + + uint8_t *dst = (uint8_t *)malloc(totalBytes); + if (!dst) { + fprintf(stderr, "[READ_DATA] oom\n"); + goto fail; + } + + const uint32_t MAX_SECTORS_PER_CMD = 27; + + uint32_t remaining = sectors; + uint32_t curLBA = lba; + uint32_t written = 0; + + while (remaining > 0) { + uint32_t xfer = (remaining > MAX_SECTORS_PER_CMD) ? MAX_SECTORS_PER_CMD : remaining; + uint32_t bytes = xfer * sector_size; + + uint8_t cdb[12] = {0}; + cdb[0] = 0xBE; + cdb[1] = cdb_byte1; + cdb[2] = (uint8_t)((curLBA >> 24) & 0xFF); + cdb[3] = (uint8_t)((curLBA >> 16) & 0xFF); + cdb[4] = (uint8_t)((curLBA >> 8) & 0xFF); + cdb[5] = (uint8_t)((curLBA >> 0) & 0xFF); + cdb[6] = (uint8_t)((xfer >> 16) & 0xFF); + cdb[7] = (uint8_t)((xfer >> 8) & 0xFF); + cdb[8] = (uint8_t)((xfer >> 0) & 0xFF); + cdb[9] = cdb_byte9; + cdb[10] = 0x00; + cdb[11] = 0x00; + + SCSITaskInterface **task = (*dev)->CreateSCSITask(dev); + if (!task) { + fprintf(stderr, "[READ_DATA] CreateSCSITask failed\n"); + free(dst); + goto fail; + } + + IOVirtualRange vr = {0}; + vr.address = (IOVirtualAddress)(dst + written); + vr.length = bytes; + + if ((*task)->SetCommandDescriptorBlock(task, cdb, sizeof(cdb)) != kIOReturnSuccess) { + fprintf(stderr, "[READ_DATA] SetCommandDescriptorBlock failed\n"); + (*task)->Release(task); + free(dst); + goto fail; + } + + if ((*task)->SetScatterGatherEntries(task, &vr, 1, bytes, kSCSIDataTransfer_FromTargetToInitiator) != kIOReturnSuccess) { + fprintf(stderr, "[READ_DATA] SetScatterGatherEntries failed\n"); + (*task)->Release(task); + free(dst); + goto fail; + } + + SCSI_Sense_Data sense = {0}; + SCSITaskStatus status = kSCSITaskStatus_No_Status; + kern_return_t ex = (*task)->ExecuteTaskSync(task, &sense, &status, NULL); + (*task)->Release(task); + + if (ex != kIOReturnSuccess || status != kSCSITaskStatus_GOOD) { + fill_data_scsi_error(outErr, ex, status, &sense); + fprintf(stderr, "[READ_DATA] ExecuteTaskSync failed (ex=0x%x, status=%u)\n", ex, status); + free(dst); + goto fail; + } + + written += bytes; + curLBA += xfer; + remaining -= xfer; + } + + *outBuf = dst; + *outLen = written; + + return true; + +fail: + return false; +} diff --git a/src/mac/device_service.c b/src/mac/device_service.c index 700f9f9..2550ffb 100644 --- a/src/mac/device_service.c +++ b/src/mac/device_service.c @@ -1,6 +1,8 @@ #include "shim_common.h" char globalBsdName[64] = {0}; +SCSITaskDeviceInterface **globalDev = NULL; +static IOCFPlugInInterface **globalPlugIn = NULL; int open_cd_raw_device(void) { if (globalBsdName[0] == '\0') { @@ -28,13 +30,90 @@ Boolean open_dev_session(const char *bsdName) { } if (globalBsdName[0] != '\0') { - return strcmp(globalBsdName, bsdName) == 0; + if (strcmp(globalBsdName, bsdName) == 0) { + return true; + } + close_dev_session(); } snprintf(globalBsdName, sizeof(globalBsdName), "%s", bsdName); + + CFMutableDictionaryRef matching = IOServiceMatching(kIOMediaClass); + if (!matching) return false; + + CFStringRef cfBsdName = CFStringCreateWithCString(kCFAllocatorDefault, bsdName, kCFStringEncodingUTF8); + if (!cfBsdName) { + CFRelease(matching); + return false; + } + CFDictionaryAddValue(matching, CFSTR(kIOBSDNameKey), cfBsdName); + CFRelease(cfBsdName); + + io_service_t service = IOServiceGetMatchingService(kIOMasterPortDefault, matching); + if (!service) return false; + + io_service_t taskDevice = IO_OBJECT_NULL; + io_service_t current = service; + IOObjectRetain(current); + + while (current) { + if (IOObjectConformsTo(current, "SCSITaskDevice")) { + taskDevice = current; + break; + } + io_service_t parent = IO_OBJECT_NULL; + kern_return_t kr = IORegistryEntryGetParentEntry(current, kIOServicePlane, &parent); + IOObjectRelease(current); + if (kr != KERN_SUCCESS) { + current = IO_OBJECT_NULL; + } else { + current = parent; + } + } + + if (!taskDevice) { + IOObjectRelease(service); + return false; + } + + S32 score = 0; + kern_return_t kr = IOCreatePlugInInterfaceForService( + taskDevice, + kSCSITaskDeviceUserClientTypeID, + kIOCFPlugInInterfaceID, + &globalPlugIn, + &score + ); + IOObjectRelease(taskDevice); + IOObjectRelease(service); + + if (kr != KERN_SUCCESS || !globalPlugIn) { + return false; + } + + kr = (*globalPlugIn)->QueryInterface( + globalPlugIn, + CFUUIDGetUUIDBytes(kSCSITaskDeviceInterfaceID), + (LPVOID *)&globalDev + ); + + if (kr != KERN_SUCCESS || !globalDev) { + IODestroyPlugInInterface(globalPlugIn); + globalPlugIn = NULL; + return false; + } + return true; } void close_dev_session(void) { + if (globalDev) { + (*globalDev)->Release(globalDev); + globalDev = NULL; + } + if (globalPlugIn) { + IODestroyPlugInInterface(globalPlugIn); + globalPlugIn = NULL; + } globalBsdName[0] = '\0'; } diff --git a/src/mac/shim_common.h b/src/mac/shim_common.h index 81bd712..6841596 100644 --- a/src/mac/shim_common.h +++ b/src/mac/shim_common.h @@ -7,6 +7,8 @@ #import #import #import +#import +#import #include #include @@ -35,9 +37,11 @@ typedef struct { } CdDriveInfo; extern char globalBsdName[64]; +extern SCSITaskDeviceInterface **globalDev; bool cd_read_toc(uint8_t **outBuf, uint32_t *outLen, CdScsiError *outErr); bool read_cd_audio(uint32_t lba, uint32_t sectors, uint8_t **outBuf, uint32_t *outLen, CdScsiError *outErr); +bool read_cd_data(uint32_t lba, uint32_t sectors, uint8_t cdb_byte1, uint8_t cdb_byte9, uint32_t sector_size, uint8_t **outBuf, uint32_t *outLen, CdScsiError *outErr); void cd_free(void *p); bool list_cd_drives(CdDriveInfo **outDrives, uint32_t *outCount); diff --git a/src/macos.rs b/src/macos.rs index d66e6d9..6d4076b 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -38,6 +38,16 @@ unsafe extern "C" { out_len: *mut u32, out_err: *mut MacScsiError, ) -> bool; + fn read_cd_data( + lba: u32, + sectors: u32, + cdb_byte1: u8, + cdb_byte9: u8, + sector_size: u32, + out_buf: *mut *mut u8, + out_len: *mut u32, + out_err: *mut MacScsiError, + ) -> bool; fn cd_free(p: *mut libc::c_void); fn list_cd_drives(out_drives: *mut *mut MacDriveInfo, out_count: *mut u32) -> bool; fn open_dev_session(bsd_name: *const libc::c_char) -> bool; @@ -223,6 +233,37 @@ fn map_mac_error( CdReaderError::Io(std::io::Error::other("macOS CD command failed")) } +pub(crate) fn read_data_chunk( + lba: u32, + sectors: u32, + mode: &crate::data_reader::SectorReadMode, +) -> Result, CdReaderError> { + let mut buf: *mut u8 = ptr::null_mut(); + let mut len: u32 = 0; + let mut err: MacScsiError = Default::default(); + let ok = unsafe { + read_cd_data( + lba, + sectors, + mode.cdb_byte1(), + mode.cdb_byte9(), + mode.sector_size() as u32, + &mut buf, + &mut len, + &mut err, + ) + }; + + if !ok { + return Err(map_mac_error(err, ScsiOp::ReadCd, Some(lba), Some(sectors))); + } + + let data = unsafe { slice::from_raw_parts(buf, len as usize) }; + let result = data.to_vec(); + unsafe { cd_free(buf as *mut _) }; + Ok(result) +} + fn next_chunk_size(current: u32, min_chunk: u32) -> u32 { if current > 8 { 8.max(min_chunk) diff --git a/src/windows.rs b/src/windows.rs index 4f14699..0fa56da 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -78,6 +78,17 @@ pub fn open_drive(path: &str) -> std::io::Result<()> { Ok(()) } +pub(crate) fn get_drive_handle() -> Result { + unsafe { + DRIVE_HANDLE.ok_or_else(|| { + CdReaderError::Io(std::io::Error::new( + std::io::ErrorKind::NotFound, + "Drive not opened", + )) + }) + } +} + pub fn close_drive() { unsafe { if let Some(current_drive) = DRIVE_HANDLE { From 983a759422be22d5991c96dc97634341702d2235 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 5 May 2026 13:42:56 +0000 Subject: [PATCH 2/7] Fix macOS build errors and initialize SCSI task device - Replaced `kIOMediaClass` with `kIOCDMediaClass` for compatibility. - Replaced deprecated `kIOMasterPortDefault` with `kIOMainPortDefault`. - Replaced undeclared `S32` with the correct `SInt32` type. - Retained previous fixes: added missing SCSI headers, implemented device session initialization by traversing IORegistry, and corrected SCSI transfer direction for READ CD. - These changes ensure the new data reader works correctly on modern macOS versions. Co-authored-by: danifunker <1643018+danifunker@users.noreply.github.com> --- src/mac/device_service.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mac/device_service.c b/src/mac/device_service.c index 2550ffb..f8ecde8 100644 --- a/src/mac/device_service.c +++ b/src/mac/device_service.c @@ -38,7 +38,7 @@ Boolean open_dev_session(const char *bsdName) { snprintf(globalBsdName, sizeof(globalBsdName), "%s", bsdName); - CFMutableDictionaryRef matching = IOServiceMatching(kIOMediaClass); + CFMutableDictionaryRef matching = IOServiceMatching(kIOCDMediaClass); if (!matching) return false; CFStringRef cfBsdName = CFStringCreateWithCString(kCFAllocatorDefault, bsdName, kCFStringEncodingUTF8); @@ -49,7 +49,7 @@ Boolean open_dev_session(const char *bsdName) { CFDictionaryAddValue(matching, CFSTR(kIOBSDNameKey), cfBsdName); CFRelease(cfBsdName); - io_service_t service = IOServiceGetMatchingService(kIOMasterPortDefault, matching); + io_service_t service = IOServiceGetMatchingService(kIOMainPortDefault, matching); if (!service) return false; io_service_t taskDevice = IO_OBJECT_NULL; @@ -76,7 +76,7 @@ Boolean open_dev_session(const char *bsdName) { return false; } - S32 score = 0; + SInt32 score = 0; kern_return_t kr = IOCreatePlugInInterfaceForService( taskDevice, kSCSITaskDeviceUserClientTypeID, From 195af495592281e83ba2cdb5cd7441bde811d247 Mon Sep 17 00:00:00 2001 From: Dani Sarfati Date: Wed, 27 May 2026 12:03:37 -0400 Subject: [PATCH 3/7] Refactor data sector reading to reuse existing track-reading infrastructure Instead of duplicating the retry loop, chunk reader, parse_sense, and next_chunk_size across a parallel data_reader module, parameterize the existing per-platform read functions with SectorReadMode. This removes ~260 lines of duplication and gives streaming support for data tracks for free. Co-Authored-By: Claude Opus 4.7 --- src/data_reader.rs | 276 +------------------------------------- src/lib.rs | 30 ++++- src/linux.rs | 34 ++--- src/mac/data_reader.c | 103 ++++++-------- src/macos.rs | 75 +++++------ src/windows.rs | 12 +- src/windows_read_track.rs | 29 ++-- 7 files changed, 150 insertions(+), 409 deletions(-) diff --git a/src/data_reader.rs b/src/data_reader.rs index c5ac86e..3f41a70 100644 --- a/src/data_reader.rs +++ b/src/data_reader.rs @@ -42,279 +42,11 @@ impl SectorReadMode { SectorReadMode::DataRaw => 0xF8, } } -} - -/// Read sectors from the disc in the specified mode with retry logic. -/// -/// This parallels the existing `read_sectors_with_retry` but allows choosing -/// between audio, cooked data, and raw data sector formats. -pub fn read_data_sectors( - lba: u32, - sectors: u32, - mode: SectorReadMode, - cfg: &crate::RetryConfig, -) -> Result, crate::CdReaderError> { - let sector_size = mode.sector_size(); - let max_sectors_per_xfer: u32 = match sector_size { - 2048 => 32, // 32 * 2048 = 65536 bytes - _ => 27, // 27 * 2352 = 63504 bytes - }; - - let total_bytes = (sectors as usize) * sector_size; - let mut out = Vec::::with_capacity(total_bytes); - - let mut remaining = sectors; - let mut cur_lba = lba; - let attempts_total = cfg.max_attempts.max(1); - - while remaining > 0 { - let mut chunk_sectors = remaining.min(max_sectors_per_xfer); - let min_chunk = cfg.min_sectors_per_read.max(1); - let mut backoff_ms = cfg.initial_backoff_ms; - let mut last_err: Option = None; - for attempt in 1..=attempts_total { - match read_data_chunk(cur_lba, chunk_sectors, &mode) { - Ok(chunk) => { - out.extend_from_slice(&chunk); - cur_lba += chunk_sectors; - remaining -= chunk_sectors; - last_err = None; - break; - } - Err(err) => { - last_err = Some(err); - if attempt == attempts_total { - break; - } - if cfg.reduce_chunk_on_retry && chunk_sectors > min_chunk { - chunk_sectors = next_chunk_size(chunk_sectors, min_chunk); - } - if backoff_ms > 0 { - std::thread::sleep(std::time::Duration::from_millis(backoff_ms)); - } - if cfg.max_backoff_ms > 0 { - backoff_ms = (backoff_ms.saturating_mul(2)).min(cfg.max_backoff_ms); - } - } - } + pub(crate) fn max_sectors_per_xfer(&self) -> u32 { + match self.sector_size() { + 2048 => 32, + _ => 27, } - - if let Some(err) = last_err { - return Err(err); - } - } - - Ok(out) -} - -fn next_chunk_size(current: u32, min_chunk: u32) -> u32 { - if current > 8 { - 8.max(min_chunk) - } else { - min_chunk - } -} - -// ── Platform-specific read_data_chunk implementations ── - -#[cfg(target_os = "linux")] -fn read_data_chunk( - lba: u32, - this_sectors: u32, - mode: &SectorReadMode, -) -> Result, crate::CdReaderError> { - use crate::{CdReaderError, ScsiError, ScsiOp}; - use libc::{c_uchar, c_void}; - - let sector_size = mode.sector_size(); - let mut chunk = vec![0u8; (this_sectors as usize) * sector_size]; - let mut sense = vec![0u8; 64]; - - let mut cdb = [0u8; 12]; - cdb[0] = 0xBE; // READ CD - cdb[1] = mode.cdb_byte1(); - cdb[2] = ((lba >> 24) & 0xFF) as u8; - cdb[3] = ((lba >> 16) & 0xFF) as u8; - cdb[4] = ((lba >> 8) & 0xFF) as u8; - cdb[5] = (lba & 0xFF) as u8; - cdb[6] = ((this_sectors >> 16) & 0xFF) as u8; - cdb[7] = ((this_sectors >> 8) & 0xFF) as u8; - cdb[8] = (this_sectors & 0xFF) as u8; - cdb[9] = mode.cdb_byte9(); - - let fd = crate::linux::get_drive_fd()?; - - // Build SgIoHeader inline (same layout as linux.rs) - #[repr(C)] - struct SgIoHeader { - interface_id: i32, - dxfer_direction: i32, - cmd_len: u8, - mx_sb_len: u8, - iovec_count: u16, - dxfer_len: u32, - dxferp: *mut c_void, - cmdp: *mut c_uchar, - sbp: *mut c_uchar, - timeout: u32, - flags: u32, - pack_id: i32, - usr_ptr: *mut c_void, - status: u8, - masked_status: u8, - msg_status: u8, - sb_len_wr: u8, - host_status: u16, - driver_status: u16, - resid: i32, - duration: u32, - info: u32, - } - - const SG_DXFER_FROM_DEV: i32 = -3; - const SG_INFO_CHECK: u32 = 0x1; - const SG_IO: u64 = 0x2285; - - let mut hdr = SgIoHeader { - interface_id: b'S' as i32, - dxfer_direction: SG_DXFER_FROM_DEV, - cmd_len: cdb.len() as u8, - mx_sb_len: sense.len() as u8, - iovec_count: 0, - dxfer_len: chunk.len() as u32, - dxferp: chunk.as_mut_ptr() as *mut c_void, - cmdp: cdb.as_mut_ptr(), - sbp: sense.as_mut_ptr(), - timeout: 30_000, - flags: 0, - pack_id: 0, - usr_ptr: std::ptr::null_mut(), - status: 0, - masked_status: 0, - msg_status: 0, - sb_len_wr: 0, - host_status: 0, - driver_status: 0, - resid: 0, - duration: 0, - info: 0, - }; - - let ret = unsafe { libc::ioctl(fd, SG_IO, &mut hdr as *mut _) }; - if ret < 0 { - return Err(CdReaderError::Io(std::io::Error::last_os_error())); - } - - if hdr.info & SG_INFO_CHECK != 0 || hdr.status != 0 { - let (sense_key, asc, ascq) = parse_sense(&sense, hdr.sb_len_wr); - return Err(CdReaderError::Scsi(ScsiError { - op: ScsiOp::ReadCd, - lba: Some(lba), - sectors: Some(this_sectors), - scsi_status: hdr.status, - sense_key, - asc, - ascq, - })); - } - - if hdr.resid > 0 { - let got = (chunk.len() as i32 - hdr.resid).max(0) as usize; - chunk.truncate(got); - } - - Ok(chunk) -} - -#[cfg(target_os = "windows")] -fn read_data_chunk( - lba: u32, - this_sectors: u32, - mode: &SectorReadMode, -) -> Result, crate::CdReaderError> { - use crate::windows::SptdWithSense; - use crate::{CdReaderError, ScsiError, ScsiOp}; - use windows_sys::Win32::Storage::IscsiDisc::{ - IOCTL_SCSI_PASS_THROUGH_DIRECT, SCSI_IOCTL_DATA_IN, SCSI_PASS_THROUGH_DIRECT, - }; - use windows_sys::Win32::System::IO::DeviceIoControl; - - let sector_size = mode.sector_size(); - let mut chunk = vec![0u8; (this_sectors as usize) * sector_size]; - - let mut wrapper: SptdWithSense = unsafe { std::mem::zeroed() }; - let sptd = &mut wrapper.sptd; - - sptd.Length = size_of::() as u16; - sptd.CdbLength = 12; - sptd.DataIn = SCSI_IOCTL_DATA_IN as u8; - sptd.TimeOutValue = 30; - sptd.DataTransferLength = chunk.len() as u32; - sptd.DataBuffer = chunk.as_mut_ptr() as *mut _; - sptd.SenseInfoLength = wrapper.sense.len() as u8; - sptd.SenseInfoOffset = size_of::() as u32; - - let cdb = &mut sptd.Cdb; - cdb.fill(0); - cdb[0] = 0xBE; - cdb[1] = mode.cdb_byte1(); - cdb[2] = ((lba >> 24) & 0xFF) as u8; - cdb[3] = ((lba >> 16) & 0xFF) as u8; - cdb[4] = ((lba >> 8) & 0xFF) as u8; - cdb[5] = (lba & 0xFF) as u8; - cdb[6] = ((this_sectors >> 16) & 0xFF) as u8; - cdb[7] = ((this_sectors >> 8) & 0xFF) as u8; - cdb[8] = (this_sectors & 0xFF) as u8; - cdb[9] = mode.cdb_byte9(); - - let handle = crate::windows::get_drive_handle()?; - let mut bytes = 0u32; - let ok = unsafe { - DeviceIoControl( - handle, - IOCTL_SCSI_PASS_THROUGH_DIRECT, - &mut wrapper as *mut _ as *mut _, - size_of::() as u32, - &mut wrapper as *mut _ as *mut _, - size_of::() as u32, - &mut bytes as *mut u32, - std::ptr::null_mut(), - ) - }; - - if ok == 0 { - return Err(CdReaderError::Io(std::io::Error::last_os_error())); - } - if wrapper.sptd.ScsiStatus != 0 { - let (sense_key, asc, ascq) = parse_sense(&wrapper.sense, wrapper.sptd.SenseInfoLength); - return Err(CdReaderError::Scsi(ScsiError { - op: ScsiOp::ReadCd, - lba: Some(lba), - sectors: Some(this_sectors), - scsi_status: wrapper.sptd.ScsiStatus, - sense_key, - asc, - ascq, - })); - } - - Ok(chunk) -} - -#[cfg(target_os = "macos")] -fn read_data_chunk( - lba: u32, - this_sectors: u32, - mode: &SectorReadMode, -) -> Result, crate::CdReaderError> { - crate::macos::read_data_chunk(lba, this_sectors, mode) -} - -#[cfg(any(target_os = "linux", target_os = "windows"))] -fn parse_sense(sense: &[u8], sb_len_wr: u8) -> (Option, Option, Option) { - if sb_len_wr == 0 || sense.len() < 14 { - return (None, None, None); } - (Some(sense[2] & 0x0F), Some(sense[12]), Some(sense[13])) } diff --git a/src/lib.rs b/src/lib.rs index 108f41d..aa59250 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -335,7 +335,35 @@ impl CdReader { mode: SectorReadMode, cfg: &RetryConfig, ) -> Result, CdReaderError> { - data_reader::read_data_sectors(lba, count, mode, cfg) + self.read_sectors_with_mode(lba, count, &mode, cfg) + } + + pub(crate) fn read_sectors_with_mode( + &self, + start_lba: u32, + sectors: u32, + mode: &SectorReadMode, + cfg: &RetryConfig, + ) -> Result, CdReaderError> { + #[cfg(target_os = "windows")] + { + windows::read_sectors_with_mode(start_lba, sectors, mode, cfg) + } + + #[cfg(target_os = "macos")] + { + macos::read_sectors_with_mode(start_lba, sectors, mode, cfg) + } + + #[cfg(target_os = "linux")] + { + linux::read_sectors_with_mode(start_lba, sectors, mode, cfg) + } + + #[cfg(not(any(target_os = "windows", target_os = "linux", target_os = "macos")))] + { + compile_error!("Unsupported platform") + } } pub(crate) fn read_sectors_with_retry( diff --git a/src/linux.rs b/src/linux.rs index 0769135..96960c9 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -9,6 +9,7 @@ use std::thread::sleep; use std::time::Duration; use crate::Toc; +use crate::data_reader::SectorReadMode; use crate::parse_toc::parse_toc; use crate::utils::get_track_bounds; use crate::{CdReaderError, RetryConfig, ScsiError, ScsiOp}; @@ -215,23 +216,19 @@ pub fn read_sectors_with_retry( sectors: u32, cfg: &RetryConfig, ) -> std::result::Result, CdReaderError> { - read_cd_audio_range(start_lba, sectors, cfg) + read_sectors_with_mode(start_lba, sectors, &SectorReadMode::Audio, cfg) } -// --- READ CD (0xBE): read an arbitrary LBA range as CD-DA (2352 bytes/sector) --- -fn read_cd_audio_range( +pub fn read_sectors_with_mode( start_lba: u32, sectors: u32, + mode: &SectorReadMode, cfg: &RetryConfig, ) -> std::result::Result, CdReaderError> { - // SCSI-2 defines reading data in 2352 bytes chunks - const SECTOR_BYTES: usize = 2352; + let sector_size = mode.sector_size(); + let max_sectors_per_xfer = mode.max_sectors_per_xfer(); - // read ~64 KBs per request - const MAX_SECTORS_PER_XFER: u32 = 27; // 27 * 2352 = 63,504 bytes - - let total_bytes = (sectors as usize) * SECTOR_BYTES; - // allocate the entire necessary size from the beginning to avoid memory realloc + let total_bytes = (sectors as usize) * sector_size; let mut out = Vec::::with_capacity(total_bytes); let mut remaining = sectors; @@ -239,13 +236,13 @@ fn read_cd_audio_range( let attempts_total = cfg.max_attempts.max(1); while remaining > 0 { - let mut chunk_sectors = min(remaining, MAX_SECTORS_PER_XFER); + let mut chunk_sectors = min(remaining, max_sectors_per_xfer); let min_chunk = cfg.min_sectors_per_read.max(1); let mut backoff_ms = cfg.initial_backoff_ms; let mut last_err: Option = None; for attempt in 1..=attempts_total { - match read_cd_audio_chunk(lba, chunk_sectors) { + match read_cd_chunk(lba, chunk_sectors, mode) { Ok(chunk) => { out.extend_from_slice(&chunk); lba += chunk_sectors; @@ -278,15 +275,14 @@ fn read_cd_audio_range( Ok(out) } -fn read_cd_audio_chunk(lba: u32, this_sectors: u32) -> std::result::Result, CdReaderError> { - const SECTOR_BYTES: usize = 2352; - let mut chunk = vec![0u8; (this_sectors as usize) * SECTOR_BYTES]; +fn read_cd_chunk(lba: u32, this_sectors: u32, mode: &SectorReadMode) -> std::result::Result, CdReaderError> { + let sector_size = mode.sector_size(); + let mut chunk = vec![0u8; (this_sectors as usize) * sector_size]; let mut sense = vec![0u8; 64]; - // CDB: READ CD (0xBE), LBA addressing let mut cdb = [0u8; 12]; - cdb.fill(0); cdb[0] = 0xBE; // READ CD + cdb[1] = mode.cdb_byte1(); cdb[2] = ((lba >> 24) & 0xFF) as u8; cdb[3] = ((lba >> 16) & 0xFF) as u8; cdb[4] = ((lba >> 8) & 0xFF) as u8; @@ -294,9 +290,7 @@ fn read_cd_audio_chunk(lba: u32, this_sectors: u32) -> std::result::Result> 16) & 0xFF) as u8; cdb[7] = ((this_sectors >> 8) & 0xFF) as u8; cdb[8] = (this_sectors & 0xFF) as u8; - cdb[9] = 0x10; - cdb[10] = 0x00; - cdb[11] = 0x00; + cdb[9] = mode.cdb_byte9(); let mut hdr = SgIoHeader { interface_id: 'S' as i32, diff --git a/src/mac/data_reader.c b/src/mac/data_reader.c index dc9ec77..cc56f46 100644 --- a/src/mac/data_reader.c +++ b/src/mac/data_reader.c @@ -70,74 +70,59 @@ bool read_cd_data(uint32_t lba, uint32_t sectors, goto fail; } - const uint32_t MAX_SECTORS_PER_CMD = 27; - - uint32_t remaining = sectors; - uint32_t curLBA = lba; - uint32_t written = 0; - - while (remaining > 0) { - uint32_t xfer = (remaining > MAX_SECTORS_PER_CMD) ? MAX_SECTORS_PER_CMD : remaining; - uint32_t bytes = xfer * sector_size; - - uint8_t cdb[12] = {0}; - cdb[0] = 0xBE; - cdb[1] = cdb_byte1; - cdb[2] = (uint8_t)((curLBA >> 24) & 0xFF); - cdb[3] = (uint8_t)((curLBA >> 16) & 0xFF); - cdb[4] = (uint8_t)((curLBA >> 8) & 0xFF); - cdb[5] = (uint8_t)((curLBA >> 0) & 0xFF); - cdb[6] = (uint8_t)((xfer >> 16) & 0xFF); - cdb[7] = (uint8_t)((xfer >> 8) & 0xFF); - cdb[8] = (uint8_t)((xfer >> 0) & 0xFF); - cdb[9] = cdb_byte9; - cdb[10] = 0x00; - cdb[11] = 0x00; - - SCSITaskInterface **task = (*dev)->CreateSCSITask(dev); - if (!task) { - fprintf(stderr, "[READ_DATA] CreateSCSITask failed\n"); - free(dst); - goto fail; - } + uint8_t cdb[12] = {0}; + cdb[0] = 0xBE; + cdb[1] = cdb_byte1; + cdb[2] = (uint8_t)((lba >> 24) & 0xFF); + cdb[3] = (uint8_t)((lba >> 16) & 0xFF); + cdb[4] = (uint8_t)((lba >> 8) & 0xFF); + cdb[5] = (uint8_t)((lba >> 0) & 0xFF); + cdb[6] = (uint8_t)((sectors >> 16) & 0xFF); + cdb[7] = (uint8_t)((sectors >> 8) & 0xFF); + cdb[8] = (uint8_t)((sectors >> 0) & 0xFF); + cdb[9] = cdb_byte9; + cdb[10] = 0x00; + cdb[11] = 0x00; + + SCSITaskInterface **task = (*dev)->CreateSCSITask(dev); + if (!task) { + fprintf(stderr, "[READ_DATA] CreateSCSITask failed\n"); + free(dst); + goto fail; + } - IOVirtualRange vr = {0}; - vr.address = (IOVirtualAddress)(dst + written); - vr.length = bytes; + IOVirtualRange vr = {0}; + vr.address = (IOVirtualAddress)dst; + vr.length = totalBytes; - if ((*task)->SetCommandDescriptorBlock(task, cdb, sizeof(cdb)) != kIOReturnSuccess) { - fprintf(stderr, "[READ_DATA] SetCommandDescriptorBlock failed\n"); - (*task)->Release(task); - free(dst); - goto fail; - } - - if ((*task)->SetScatterGatherEntries(task, &vr, 1, bytes, kSCSIDataTransfer_FromTargetToInitiator) != kIOReturnSuccess) { - fprintf(stderr, "[READ_DATA] SetScatterGatherEntries failed\n"); - (*task)->Release(task); - free(dst); - goto fail; - } + if ((*task)->SetCommandDescriptorBlock(task, cdb, sizeof(cdb)) != kIOReturnSuccess) { + fprintf(stderr, "[READ_DATA] SetCommandDescriptorBlock failed\n"); + (*task)->Release(task); + free(dst); + goto fail; + } - SCSI_Sense_Data sense = {0}; - SCSITaskStatus status = kSCSITaskStatus_No_Status; - kern_return_t ex = (*task)->ExecuteTaskSync(task, &sense, &status, NULL); + if ((*task)->SetScatterGatherEntries(task, &vr, 1, totalBytes, kSCSIDataTransfer_FromTargetToInitiator) != kIOReturnSuccess) { + fprintf(stderr, "[READ_DATA] SetScatterGatherEntries failed\n"); (*task)->Release(task); + free(dst); + goto fail; + } - if (ex != kIOReturnSuccess || status != kSCSITaskStatus_GOOD) { - fill_data_scsi_error(outErr, ex, status, &sense); - fprintf(stderr, "[READ_DATA] ExecuteTaskSync failed (ex=0x%x, status=%u)\n", ex, status); - free(dst); - goto fail; - } + SCSI_Sense_Data sense = {0}; + SCSITaskStatus status = kSCSITaskStatus_No_Status; + kern_return_t ex = (*task)->ExecuteTaskSync(task, &sense, &status, NULL); + (*task)->Release(task); - written += bytes; - curLBA += xfer; - remaining -= xfer; + if (ex != kIOReturnSuccess || status != kSCSITaskStatus_GOOD) { + fill_data_scsi_error(outErr, ex, status, &sense); + fprintf(stderr, "[READ_DATA] ExecuteTaskSync failed (ex=0x%x, status=%u)\n", ex, status); + free(dst); + goto fail; } *outBuf = dst; - *outLen = written; + *outLen = totalBytes; return true; diff --git a/src/macos.rs b/src/macos.rs index 6d4076b..6141140 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -5,6 +5,7 @@ use std::{thread::sleep, time::Duration}; use crate::parse_toc::parse_toc; use crate::utils::get_track_bounds; +use crate::data_reader::SectorReadMode; use crate::{CdReaderError, DriveInfo, RetryConfig, ScsiError, ScsiOp, Toc}; #[repr(C)] @@ -143,22 +144,31 @@ pub fn read_sectors_with_retry( sectors: u32, cfg: &RetryConfig, ) -> Result, CdReaderError> { - const SECTOR_BYTES: usize = 2352; - const MAX_SECTORS_PER_XFER: u32 = 27; + read_sectors_with_mode(start_lba, sectors, &SectorReadMode::Audio, cfg) +} + +pub fn read_sectors_with_mode( + start_lba: u32, + sectors: u32, + mode: &SectorReadMode, + cfg: &RetryConfig, +) -> Result, CdReaderError> { + let sector_size = mode.sector_size(); + let max_sectors_per_xfer = mode.max_sectors_per_xfer(); - let mut out = Vec::::with_capacity((sectors as usize) * SECTOR_BYTES); + let mut out = Vec::::with_capacity((sectors as usize) * sector_size); let mut remaining = sectors; let mut lba = start_lba; let attempts_total = cfg.max_attempts.max(1); let min_chunk = cfg.min_sectors_per_read.max(1); while remaining > 0 { - let mut chunk_sectors = remaining.min(MAX_SECTORS_PER_XFER); + let mut chunk_sectors = remaining.min(max_sectors_per_xfer); let mut backoff_ms = cfg.initial_backoff_ms; let mut last_err: Option = None; for attempt in 1..=attempts_total { - match read_cd_audio_chunk(lba, chunk_sectors) { + match read_cd_chunk(lba, chunk_sectors, mode) { Ok(chunk) => { out.extend_from_slice(&chunk); lba += chunk_sectors; @@ -192,21 +202,35 @@ pub fn read_sectors_with_retry( Ok(out) } -fn read_cd_audio_chunk(lba: u32, sectors: u32) -> Result, CdReaderError> { +fn read_cd_chunk(lba: u32, sectors: u32, mode: &SectorReadMode) -> Result, CdReaderError> { let mut buf: *mut u8 = ptr::null_mut(); let mut len: u32 = 0; let mut err: MacScsiError = Default::default(); - let ok = unsafe { read_cd_audio(lba, sectors, &mut buf, &mut len, &mut err) }; + + let ok = match mode { + SectorReadMode::Audio => unsafe { + read_cd_audio(lba, sectors, &mut buf, &mut len, &mut err) + }, + _ => unsafe { + read_cd_data( + lba, + sectors, + mode.cdb_byte1(), + mode.cdb_byte9(), + mode.sector_size() as u32, + &mut buf, + &mut len, + &mut err, + ) + }, + }; if !ok { return Err(map_mac_error(err, ScsiOp::ReadCd, Some(lba), Some(sectors))); } let data = unsafe { slice::from_raw_parts(buf, len as usize) }; - - // `.to_vec()` will copy the data, so we can free it safely after let result = data.to_vec(); - unsafe { cd_free(buf as *mut _) }; Ok(result) @@ -233,37 +257,6 @@ fn map_mac_error( CdReaderError::Io(std::io::Error::other("macOS CD command failed")) } -pub(crate) fn read_data_chunk( - lba: u32, - sectors: u32, - mode: &crate::data_reader::SectorReadMode, -) -> Result, CdReaderError> { - let mut buf: *mut u8 = ptr::null_mut(); - let mut len: u32 = 0; - let mut err: MacScsiError = Default::default(); - let ok = unsafe { - read_cd_data( - lba, - sectors, - mode.cdb_byte1(), - mode.cdb_byte9(), - mode.sector_size() as u32, - &mut buf, - &mut len, - &mut err, - ) - }; - - if !ok { - return Err(map_mac_error(err, ScsiOp::ReadCd, Some(lba), Some(sectors))); - } - - let data = unsafe { slice::from_raw_parts(buf, len as usize) }; - let result = data.to_vec(); - unsafe { cd_free(buf as *mut _) }; - Ok(result) -} - fn next_chunk_size(current: u32, min_chunk: u32) -> u32 { if current > 8 { 8.max(min_chunk) diff --git a/src/windows.rs b/src/windows.rs index 0fa56da..1f1fb5e 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -10,6 +10,7 @@ use windows_sys::Win32::Storage::IscsiDisc::{ }; use windows_sys::Win32::System::IO::DeviceIoControl; +use crate::data_reader::SectorReadMode; use crate::{CdReaderError, RetryConfig, ScsiError, ScsiOp, Toc, parse_toc, windows_read_track}; use std::mem; @@ -185,13 +186,22 @@ pub fn read_sectors_with_retry( start_lba: u32, sectors: u32, cfg: &RetryConfig, +) -> Result, CdReaderError> { + read_sectors_with_mode(start_lba, sectors, &SectorReadMode::Audio, cfg) +} + +pub fn read_sectors_with_mode( + start_lba: u32, + sectors: u32, + mode: &SectorReadMode, + cfg: &RetryConfig, ) -> Result, CdReaderError> { let handle = unsafe { DRIVE_HANDLE .ok_or_else(|| std::io::Error::new(std::io::ErrorKind::NotFound, "Drive not opened")) .map_err(CdReaderError::Io)? }; - windows_read_track::read_audio_range_with_retry(handle, start_lba, sectors, cfg) + windows_read_track::read_range_with_retry(handle, start_lba, sectors, mode, cfg) } fn parse_sense(sense: &[u8], sense_len: u8) -> (Option, Option, Option) { diff --git a/src/windows_read_track.rs b/src/windows_read_track.rs index 6c9102d..c5898e3 100644 --- a/src/windows_read_track.rs +++ b/src/windows_read_track.rs @@ -11,23 +11,20 @@ use windows_sys::Win32::Storage::IscsiDisc::{ use windows_sys::Win32::System::IO::DeviceIoControl; use crate::windows::SptdWithSense; +use crate::data_reader::SectorReadMode; use crate::{CdReaderError, RetryConfig, ScsiError, ScsiOp}; -// --- READ CD (0xBE): read an arbitrary LBA range as CD-DA (2352 bytes/sector) --- -pub fn read_audio_range_with_retry( +pub fn read_range_with_retry( handle: HANDLE, start_lba: u32, sectors: u32, + mode: &SectorReadMode, cfg: &RetryConfig, ) -> Result, CdReaderError> { - // SCSI-2 defines reading data in 2352 bytes chunks - const SECTOR_BYTES: usize = 2352; + let sector_size = mode.sector_size(); + let max_sectors_per_xfer = mode.max_sectors_per_xfer(); - // read ~64 KBs per request - const MAX_SECTORS_PER_XFER: u32 = 27; // 27 * 2352 = 63,504 bytes - - let total_bytes = (sectors as usize) * SECTOR_BYTES; - // allocate the entire necessary size from the beginning to avoid memory realloc + let total_bytes = (sectors as usize) * sector_size; let mut out = Vec::::with_capacity(total_bytes); let mut remaining = sectors; @@ -35,13 +32,13 @@ pub fn read_audio_range_with_retry( let attempts_total = cfg.max_attempts.max(1); while remaining > 0 { - let mut chunk_sectors = min(remaining, MAX_SECTORS_PER_XFER); + let mut chunk_sectors = min(remaining, max_sectors_per_xfer); let min_chunk = cfg.min_sectors_per_read.max(1); let mut backoff_ms = cfg.initial_backoff_ms; let mut last_err: Option = None; for attempt in 1..=attempts_total { - match read_cd_audio_chunk(handle, lba, chunk_sectors) { + match read_cd_chunk(handle, lba, chunk_sectors, mode) { Ok(chunk) => { out.extend_from_slice(&chunk); lba += chunk_sectors; @@ -74,13 +71,14 @@ pub fn read_audio_range_with_retry( Ok(out) } -fn read_cd_audio_chunk( +fn read_cd_chunk( handle: HANDLE, lba: u32, this_sectors: u32, + mode: &SectorReadMode, ) -> Result, CdReaderError> { - const SECTOR_BYTES: usize = 2352; - let mut chunk = vec![0u8; (this_sectors as usize) * SECTOR_BYTES]; + let sector_size = mode.sector_size(); + let mut chunk = vec![0u8; (this_sectors as usize) * sector_size]; let mut wrapper: SptdWithSense = unsafe { mem::zeroed() }; let sptd = &mut wrapper.sptd; @@ -97,6 +95,7 @@ fn read_cd_audio_chunk( let cdb = &mut sptd.Cdb; cdb.fill(0); cdb[0] = 0xBE; + cdb[1] = mode.cdb_byte1(); cdb[2] = ((lba >> 24) & 0xFF) as u8; cdb[3] = ((lba >> 16) & 0xFF) as u8; cdb[4] = ((lba >> 8) & 0xFF) as u8; @@ -104,7 +103,7 @@ fn read_cd_audio_chunk( cdb[6] = ((this_sectors >> 16) & 0xFF) as u8; cdb[7] = ((this_sectors >> 8) & 0xFF) as u8; cdb[8] = (this_sectors & 0xFF) as u8; - cdb[9] = 0x10; + cdb[9] = mode.cdb_byte9(); let mut bytes = 0u32; let ok = unsafe { From a5dfafc8d29785aa3efceabf204611d5a8260cd1 Mon Sep 17 00:00:00 2001 From: Dani Sarfati Date: Wed, 27 May 2026 14:12:22 -0400 Subject: [PATCH 4/7] Fix rustfmt formatting issues (import order and function signature wrapping) Co-Authored-By: Claude Opus 4.7 --- src/linux.rs | 6 +++++- src/macos.rs | 2 +- src/windows_read_track.rs | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/linux.rs b/src/linux.rs index 96960c9..b323e9d 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -275,7 +275,11 @@ pub fn read_sectors_with_mode( Ok(out) } -fn read_cd_chunk(lba: u32, this_sectors: u32, mode: &SectorReadMode) -> std::result::Result, CdReaderError> { +fn read_cd_chunk( + lba: u32, + this_sectors: u32, + mode: &SectorReadMode, +) -> std::result::Result, CdReaderError> { let sector_size = mode.sector_size(); let mut chunk = vec![0u8; (this_sectors as usize) * sector_size]; let mut sense = vec![0u8; 64]; diff --git a/src/macos.rs b/src/macos.rs index 6141140..afd757e 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -3,9 +3,9 @@ use std::io; use std::{ptr, slice}; use std::{thread::sleep, time::Duration}; +use crate::data_reader::SectorReadMode; use crate::parse_toc::parse_toc; use crate::utils::get_track_bounds; -use crate::data_reader::SectorReadMode; use crate::{CdReaderError, DriveInfo, RetryConfig, ScsiError, ScsiOp, Toc}; #[repr(C)] diff --git a/src/windows_read_track.rs b/src/windows_read_track.rs index c5898e3..1ae4606 100644 --- a/src/windows_read_track.rs +++ b/src/windows_read_track.rs @@ -10,8 +10,8 @@ use windows_sys::Win32::Storage::IscsiDisc::{ }; use windows_sys::Win32::System::IO::DeviceIoControl; -use crate::windows::SptdWithSense; use crate::data_reader::SectorReadMode; +use crate::windows::SptdWithSense; use crate::{CdReaderError, RetryConfig, ScsiError, ScsiOp}; pub fn read_range_with_retry( From ea38302a17c3542a7d4c40e85ae638b90eea26cf Mon Sep 17 00:00:00 2001 From: Dani Sarfati Date: Thu, 28 May 2026 19:43:36 -0400 Subject: [PATCH 5/7] Remove unused get_drive_fd to fix Linux -D warnings build failure The function was dead code left from the data-reader refactor; SCSI ioctls access DRIVE_HANDLE inline. On Linux CI (-D warnings) the dead_code lint became a hard error. macOS builds never exercised this cfg path, so it passed locally while CI kept failing. Co-Authored-By: Claude Opus 4.8 --- src/linux.rs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/linux.rs b/src/linux.rs index b323e9d..38f8f11 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -86,16 +86,6 @@ pub fn open_drive(path: &str) -> std::io::Result<()> { Ok(()) } -#[allow(static_mut_refs)] -pub(crate) fn get_drive_fd() -> Result { - use std::os::fd::AsRawFd; - unsafe { - DRIVE_HANDLE.as_ref().map(|f| f.as_raw_fd()).ok_or_else(|| { - CdReaderError::Io(Error::new(std::io::ErrorKind::NotFound, "Drive not opened")) - }) - } -} - #[allow(static_mut_refs)] pub fn close_drive() { unsafe { From ddf44f5102bd1443db781883c4b71ad9387edbed Mon Sep 17 00:00:00 2001 From: Dani Sarfati Date: Thu, 28 May 2026 20:22:22 -0400 Subject: [PATCH 6/7] Unify per-platform read loop and use DKIOCCDREAD for macOS data reads Unify code paths: - Extract the shared READ CD retry/chunk/backoff loop into src/read_loop.rs as read_sectors_chunked(), taking the platform single-command read as a closure. linux.rs, macos.rs and windows_read_track.rs now keep only their platform-specific read_cd_chunk; the duplicated loop and next_chunk_size copies are removed. macOS data reads (PR review #4): - Collapse read_cd_audio + read_cd_data into one read_cd_sectors() that maps the read mode to the CD sector area/type and uses the read-only DKIOCCDREAD ioctl for audio, cooked and raw modes alike. - Delete data_reader.c, which used SCSITaskDeviceInterface and required exclusive access (forcing the volume to unmount). open_dev_session now just records the BSD name and validates it by opening the raw device. Document max_sectors_per_xfer as the sole chunker for the blocking read_track/read_data_sectors paths, kept for drive-firmware/USB-bridge reliability rather than OS pass-through limits. Verified fmt/clippy(-D warnings)/test/examples on macOS and Linux. Windows and on-disc data-read behavior (macOS DKIOCCDREAD sector-type mapping) still need hardware verification. Co-Authored-By: Claude Opus 4.8 --- build.rs | 2 - src/data_reader.rs | 15 ++++- src/lib.rs | 1 + src/linux.rs | 62 +----------------- src/mac/audio_reader.c | 53 +++++++++++++-- src/mac/data_reader.c | 131 -------------------------------------- src/mac/device_service.c | 88 +++---------------------- src/mac/shim_common.h | 6 +- src/macos.rs | 96 ++++------------------------ src/read_loop.rs | 88 +++++++++++++++++++++++++ src/windows_read_track.rs | 62 +----------------- 11 files changed, 178 insertions(+), 426 deletions(-) delete mode 100644 src/mac/data_reader.c create mode 100644 src/read_loop.rs diff --git a/build.rs b/build.rs index f930ee6..eabe876 100644 --- a/build.rs +++ b/build.rs @@ -6,7 +6,6 @@ fn main() { println!("cargo:rerun-if-changed=src/mac/list_drives.c"); println!("cargo:rerun-if-changed=src/mac/toc_reader.c"); println!("cargo:rerun-if-changed=src/mac/audio_reader.c"); - println!("cargo:rerun-if-changed=src/mac/data_reader.c"); println!("cargo:rustc-link-lib=framework=IOKit"); println!("cargo:rustc-link-lib=framework=CoreFoundation"); @@ -15,7 +14,6 @@ fn main() { .file("src/mac/list_drives.c") .file("src/mac/toc_reader.c") .file("src/mac/audio_reader.c") - .file("src/mac/data_reader.c") .include("src/mac") // force C compilation .flag("-x") diff --git a/src/data_reader.rs b/src/data_reader.rs index 3f41a70..f2435c1 100644 --- a/src/data_reader.rs +++ b/src/data_reader.rs @@ -43,10 +43,21 @@ impl SectorReadMode { } } + /// Maximum sectors per single `READ CD` command. + /// + /// This is the sole chunker for the blocking `read_track` / + /// `read_data_sectors` paths, which hand a whole track (tens of thousands + /// of sectors) straight to the read loop; the streaming API already limits + /// itself via `TrackStreamConfig::sectors_per_chunk`. The cap is not about + /// OS pass-through limits (modern SG_IO/SPTI handle far larger transfers) + /// but about optical-drive firmware and USB-bridge reliability: large + /// multi-sector `READ CD` requests are flaky across the zoo of drives. The + /// values keep each transfer around 64 KiB, matching the conventional + /// ~27-sector chunk used by cdparanoia/libcdio. pub(crate) fn max_sectors_per_xfer(&self) -> u32 { match self.sector_size() { - 2048 => 32, - _ => 27, + 2048 => 32, // 32 * 2048 = 64 KiB + _ => 27, // 27 * 2352 ≈ 62 KiB } } } diff --git a/src/lib.rs b/src/lib.rs index aa59250..549b299 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -153,6 +153,7 @@ mod windows; pub mod data_reader; mod discovery; mod errors; +mod read_loop; mod retry; mod stream; mod utils; diff --git a/src/linux.rs b/src/linux.rs index 38f8f11..255c3ab 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -1,12 +1,9 @@ use libc::{O_NONBLOCK, O_RDWR, c_uchar, c_void}; -use std::cmp::min; use std::ffi::CString; use std::fs::File; use std::io::Error; use std::os::fd::{AsRawFd, FromRawFd}; use std::path::Path; -use std::thread::sleep; -use std::time::Duration; use crate::Toc; use crate::data_reader::SectorReadMode; @@ -215,54 +212,9 @@ pub fn read_sectors_with_mode( mode: &SectorReadMode, cfg: &RetryConfig, ) -> std::result::Result, CdReaderError> { - let sector_size = mode.sector_size(); - let max_sectors_per_xfer = mode.max_sectors_per_xfer(); - - let total_bytes = (sectors as usize) * sector_size; - let mut out = Vec::::with_capacity(total_bytes); - - let mut remaining = sectors; - let mut lba = start_lba; - let attempts_total = cfg.max_attempts.max(1); - - while remaining > 0 { - let mut chunk_sectors = min(remaining, max_sectors_per_xfer); - let min_chunk = cfg.min_sectors_per_read.max(1); - let mut backoff_ms = cfg.initial_backoff_ms; - let mut last_err: Option = None; - - for attempt in 1..=attempts_total { - match read_cd_chunk(lba, chunk_sectors, mode) { - Ok(chunk) => { - out.extend_from_slice(&chunk); - lba += chunk_sectors; - remaining -= chunk_sectors; - last_err = None; - break; - } - Err(err) => { - last_err = Some(err); - if attempt == attempts_total { - break; - } - if cfg.reduce_chunk_on_retry && chunk_sectors > min_chunk { - chunk_sectors = next_chunk_size(chunk_sectors, min_chunk); - } - if backoff_ms > 0 { - sleep(Duration::from_millis(backoff_ms)); - } - if cfg.max_backoff_ms > 0 { - backoff_ms = (backoff_ms.saturating_mul(2)).min(cfg.max_backoff_ms); - } - } - } - } - if let Some(err) = last_err { - return Err(err); - } - } - - Ok(out) + crate::read_loop::read_sectors_chunked(start_lba, sectors, mode, cfg, |lba, chunk_sectors| { + read_cd_chunk(lba, chunk_sectors, mode) + }) } fn read_cd_chunk( @@ -344,11 +296,3 @@ fn parse_sense(sense: &[u8], sb_len_wr: u8) -> (Option, Option, Option u32 { - if current > 8 { - 8.max(min_chunk) - } else { - min_chunk - } -} diff --git a/src/mac/audio_reader.c b/src/mac/audio_reader.c index 92ab9f3..35bbaef 100644 --- a/src/mac/audio_reader.c +++ b/src/mac/audio_reader.c @@ -1,19 +1,60 @@ #include "shim_common.h" -bool read_cd_audio(uint32_t lba, uint32_t sectors, uint8_t **outBuf, uint32_t *outLen, CdScsiError *outErr) { +// Map our SectorReadMode discriminant to the macOS CD sector area/type and the +// resulting bytes-per-sector. Keeping the mapping here (rather than in Rust) +// means the IOKit constants only ever appear where their header is imported. +// +// 0 = Audio -> user area, CDDA, 2352 B/sector +// 1 = DataCooked -> user area, Mode 1, 2048 B/sector +// 2 = DataRaw -> sync+header+user+aux, Mode 1, 2352 B/sector +static bool sector_layout_for_mode(uint32_t mode_id, + CDSectorArea *outArea, + CDSectorType *outType, + uint32_t *outSectorSize) { + switch (mode_id) { + case 0: + *outArea = kCDSectorAreaUser; + *outType = kCDSectorTypeCDDA; + *outSectorSize = 2352; + return true; + case 1: + *outArea = kCDSectorAreaUser; + *outType = kCDSectorTypeMode1; + *outSectorSize = 2048; + return true; + case 2: + *outArea = (CDSectorArea)(kCDSectorAreaSync | kCDSectorAreaHeader | + kCDSectorAreaUser | kCDSectorAreaAuxiliary); + *outType = kCDSectorTypeMode1; + *outSectorSize = 2352; + return true; + default: + return false; + } +} + +bool read_cd_sectors(uint32_t lba, uint32_t sectors, uint32_t mode_id, + uint8_t **outBuf, uint32_t *outLen, CdScsiError *outErr) { *outBuf = NULL; *outLen = 0; if (outErr) { memset(outErr, 0, sizeof(CdScsiError)); } - const uint32_t SECTOR_SZ = 2352; + CDSectorArea sectorArea; + CDSectorType sectorType; + uint32_t sectorSize; + if (!sector_layout_for_mode(mode_id, §orArea, §orType, §orSize)) { + fprintf(stderr, "[READ] unknown sector mode %u\n", mode_id); + goto fail; + } + if (sectors == 0) { fprintf(stderr, "[READ] sectors == 0\n"); goto fail; } - uint64_t totalBytes64 = (uint64_t)SECTOR_SZ * (uint64_t)sectors; + uint64_t totalBytes64 = (uint64_t)sectorSize * (uint64_t)sectors; if (totalBytes64 > UINT32_MAX) { fprintf(stderr, "[READ] requested size too large\n"); goto fail; @@ -34,9 +75,9 @@ bool read_cd_audio(uint32_t lba, uint32_t sectors, uint8_t **outBuf, uint32_t *o } dk_cd_read_t read = {0}; - read.offset = (uint64_t)lba * (uint64_t)SECTOR_SZ; - read.sectorArea = kCDSectorAreaUser; - read.sectorType = kCDSectorTypeCDDA; + read.offset = (uint64_t)lba * (uint64_t)sectorSize; + read.sectorArea = sectorArea; + read.sectorType = sectorType; read.bufferLength = totalBytes; read.buffer = dst; diff --git a/src/mac/data_reader.c b/src/mac/data_reader.c deleted file mode 100644 index cc56f46..0000000 --- a/src/mac/data_reader.c +++ /dev/null @@ -1,131 +0,0 @@ -#include "shim_common.h" - -static uint8_t task_status_to_scsi_status(SCSITaskStatus status) { - switch (status) { - case kSCSITaskStatus_GOOD: return 0x00; - case kSCSITaskStatus_CHECK_CONDITION: return 0x02; - case kSCSITaskStatus_BUSY: return 0x08; - case kSCSITaskStatus_RESERVATION_CONFLICT: return 0x18; - case kSCSITaskStatus_TASK_SET_FULL: return 0x28; - case kSCSITaskStatus_ACA_ACTIVE: return 0x30; - default: return 0xFF; - } -} - -static void fill_data_scsi_error(CdScsiError *outErr, kern_return_t ex, SCSITaskStatus status, SCSI_Sense_Data *sense) { - if (!outErr) return; - - outErr->has_scsi_error = 1; - outErr->exec_error = (uint32_t)ex; - outErr->task_status = (uint32_t)status; - outErr->scsi_status = task_status_to_scsi_status(status); - - const uint8_t *sense_bytes = (const uint8_t *)sense; - bool has_sense = false; - for (size_t i = 0; i < sizeof(SCSI_Sense_Data); i++) { - if (sense_bytes[i] != 0) { - has_sense = true; - break; - } - } - - outErr->has_sense = has_sense ? 1 : 0; - if (has_sense && sizeof(SCSI_Sense_Data) >= 14) { - outErr->sense_key = sense_bytes[2] & 0x0F; - outErr->asc = sense_bytes[12]; - outErr->ascq = sense_bytes[13]; - } -} - -bool read_cd_data(uint32_t lba, uint32_t sectors, - uint8_t cdb_byte1, uint8_t cdb_byte9, uint32_t sector_size, - uint8_t **outBuf, uint32_t *outLen, CdScsiError *outErr) { - *outBuf = NULL; - *outLen = 0; - if (outErr) { - memset(outErr, 0, sizeof(CdScsiError)); - } - - SCSITaskDeviceInterface **dev = globalDev; - if (!dev) { - fprintf(stderr, "[READ_DATA] Device session is not open\n"); - goto fail; - } - - if (sectors == 0) { - fprintf(stderr, "[READ_DATA] sectors == 0\n"); - goto fail; - } - - uint64_t totalBytes64 = (uint64_t)sector_size * (uint64_t)sectors; - if (totalBytes64 > UINT32_MAX) { - fprintf(stderr, "[READ_DATA] requested size too large\n"); - goto fail; - } - uint32_t totalBytes = (uint32_t)totalBytes64; - - uint8_t *dst = (uint8_t *)malloc(totalBytes); - if (!dst) { - fprintf(stderr, "[READ_DATA] oom\n"); - goto fail; - } - - uint8_t cdb[12] = {0}; - cdb[0] = 0xBE; - cdb[1] = cdb_byte1; - cdb[2] = (uint8_t)((lba >> 24) & 0xFF); - cdb[3] = (uint8_t)((lba >> 16) & 0xFF); - cdb[4] = (uint8_t)((lba >> 8) & 0xFF); - cdb[5] = (uint8_t)((lba >> 0) & 0xFF); - cdb[6] = (uint8_t)((sectors >> 16) & 0xFF); - cdb[7] = (uint8_t)((sectors >> 8) & 0xFF); - cdb[8] = (uint8_t)((sectors >> 0) & 0xFF); - cdb[9] = cdb_byte9; - cdb[10] = 0x00; - cdb[11] = 0x00; - - SCSITaskInterface **task = (*dev)->CreateSCSITask(dev); - if (!task) { - fprintf(stderr, "[READ_DATA] CreateSCSITask failed\n"); - free(dst); - goto fail; - } - - IOVirtualRange vr = {0}; - vr.address = (IOVirtualAddress)dst; - vr.length = totalBytes; - - if ((*task)->SetCommandDescriptorBlock(task, cdb, sizeof(cdb)) != kIOReturnSuccess) { - fprintf(stderr, "[READ_DATA] SetCommandDescriptorBlock failed\n"); - (*task)->Release(task); - free(dst); - goto fail; - } - - if ((*task)->SetScatterGatherEntries(task, &vr, 1, totalBytes, kSCSIDataTransfer_FromTargetToInitiator) != kIOReturnSuccess) { - fprintf(stderr, "[READ_DATA] SetScatterGatherEntries failed\n"); - (*task)->Release(task); - free(dst); - goto fail; - } - - SCSI_Sense_Data sense = {0}; - SCSITaskStatus status = kSCSITaskStatus_No_Status; - kern_return_t ex = (*task)->ExecuteTaskSync(task, &sense, &status, NULL); - (*task)->Release(task); - - if (ex != kIOReturnSuccess || status != kSCSITaskStatus_GOOD) { - fill_data_scsi_error(outErr, ex, status, &sense); - fprintf(stderr, "[READ_DATA] ExecuteTaskSync failed (ex=0x%x, status=%u)\n", ex, status); - free(dst); - goto fail; - } - - *outBuf = dst; - *outLen = totalBytes; - - return true; - -fail: - return false; -} diff --git a/src/mac/device_service.c b/src/mac/device_service.c index f8ecde8..a5e7fa3 100644 --- a/src/mac/device_service.c +++ b/src/mac/device_service.c @@ -1,8 +1,6 @@ #include "shim_common.h" char globalBsdName[64] = {0}; -SCSITaskDeviceInterface **globalDev = NULL; -static IOCFPlugInInterface **globalPlugIn = NULL; int open_cd_raw_device(void) { if (globalBsdName[0] == '\0') { @@ -24,96 +22,28 @@ int open_cd_raw_device(void) { return open(path, O_RDONLY | O_NONBLOCK); } +// Reading the TOC and sector data both go through the read-only +// DKIOCCDREAD/DKIOCCDREADTOC ioctls on the raw BSD device, so opening a +// "session" is just remembering which device to open. This avoids the +// SCSITaskDeviceInterface path, which requires exclusive access and forces +// the volume to unmount. We validate the name by opening the raw device once. Boolean open_dev_session(const char *bsdName) { if (!bsdName || bsdName[0] == '\0') { return false; } - if (globalBsdName[0] != '\0') { - if (strcmp(globalBsdName, bsdName) == 0) { - return true; - } - close_dev_session(); - } - snprintf(globalBsdName, sizeof(globalBsdName), "%s", bsdName); - CFMutableDictionaryRef matching = IOServiceMatching(kIOCDMediaClass); - if (!matching) return false; - - CFStringRef cfBsdName = CFStringCreateWithCString(kCFAllocatorDefault, bsdName, kCFStringEncodingUTF8); - if (!cfBsdName) { - CFRelease(matching); - return false; - } - CFDictionaryAddValue(matching, CFSTR(kIOBSDNameKey), cfBsdName); - CFRelease(cfBsdName); - - io_service_t service = IOServiceGetMatchingService(kIOMainPortDefault, matching); - if (!service) return false; - - io_service_t taskDevice = IO_OBJECT_NULL; - io_service_t current = service; - IOObjectRetain(current); - - while (current) { - if (IOObjectConformsTo(current, "SCSITaskDevice")) { - taskDevice = current; - break; - } - io_service_t parent = IO_OBJECT_NULL; - kern_return_t kr = IORegistryEntryGetParentEntry(current, kIOServicePlane, &parent); - IOObjectRelease(current); - if (kr != KERN_SUCCESS) { - current = IO_OBJECT_NULL; - } else { - current = parent; - } - } - - if (!taskDevice) { - IOObjectRelease(service); - return false; - } - - SInt32 score = 0; - kern_return_t kr = IOCreatePlugInInterfaceForService( - taskDevice, - kSCSITaskDeviceUserClientTypeID, - kIOCFPlugInInterfaceID, - &globalPlugIn, - &score - ); - IOObjectRelease(taskDevice); - IOObjectRelease(service); - - if (kr != KERN_SUCCESS || !globalPlugIn) { - return false; - } - - kr = (*globalPlugIn)->QueryInterface( - globalPlugIn, - CFUUIDGetUUIDBytes(kSCSITaskDeviceInterfaceID), - (LPVOID *)&globalDev - ); - - if (kr != KERN_SUCCESS || !globalDev) { - IODestroyPlugInInterface(globalPlugIn); - globalPlugIn = NULL; + int fd = open_cd_raw_device(); + if (fd < 0) { + globalBsdName[0] = '\0'; return false; } + close(fd); return true; } void close_dev_session(void) { - if (globalDev) { - (*globalDev)->Release(globalDev); - globalDev = NULL; - } - if (globalPlugIn) { - IODestroyPlugInInterface(globalPlugIn); - globalPlugIn = NULL; - } globalBsdName[0] = '\0'; } diff --git a/src/mac/shim_common.h b/src/mac/shim_common.h index 6841596..f6c1cd0 100644 --- a/src/mac/shim_common.h +++ b/src/mac/shim_common.h @@ -7,8 +7,6 @@ #import #import #import -#import -#import #include #include @@ -37,11 +35,9 @@ typedef struct { } CdDriveInfo; extern char globalBsdName[64]; -extern SCSITaskDeviceInterface **globalDev; bool cd_read_toc(uint8_t **outBuf, uint32_t *outLen, CdScsiError *outErr); -bool read_cd_audio(uint32_t lba, uint32_t sectors, uint8_t **outBuf, uint32_t *outLen, CdScsiError *outErr); -bool read_cd_data(uint32_t lba, uint32_t sectors, uint8_t cdb_byte1, uint8_t cdb_byte9, uint32_t sector_size, uint8_t **outBuf, uint32_t *outLen, CdScsiError *outErr); +bool read_cd_sectors(uint32_t lba, uint32_t sectors, uint32_t mode_id, uint8_t **outBuf, uint32_t *outLen, CdScsiError *outErr); void cd_free(void *p); bool list_cd_drives(CdDriveInfo **outDrives, uint32_t *outCount); diff --git a/src/macos.rs b/src/macos.rs index afd757e..6c4ec31 100644 --- a/src/macos.rs +++ b/src/macos.rs @@ -1,7 +1,6 @@ use std::ffi::{CStr, CString}; use std::io; use std::{ptr, slice}; -use std::{thread::sleep, time::Duration}; use crate::data_reader::SectorReadMode; use crate::parse_toc::parse_toc; @@ -32,19 +31,10 @@ struct MacDriveInfo { #[link(name = "macos_cd_shim", kind = "static")] unsafe extern "C" { fn cd_read_toc(out_buf: *mut *mut u8, out_len: *mut u32, out_err: *mut MacScsiError) -> bool; - fn read_cd_audio( + fn read_cd_sectors( lba: u32, sectors: u32, - out_buf: *mut *mut u8, - out_len: *mut u32, - out_err: *mut MacScsiError, - ) -> bool; - fn read_cd_data( - lba: u32, - sectors: u32, - cdb_byte1: u8, - cdb_byte9: u8, - sector_size: u32, + mode_id: u32, out_buf: *mut *mut u8, out_len: *mut u32, out_err: *mut MacScsiError, @@ -153,53 +143,9 @@ pub fn read_sectors_with_mode( mode: &SectorReadMode, cfg: &RetryConfig, ) -> Result, CdReaderError> { - let sector_size = mode.sector_size(); - let max_sectors_per_xfer = mode.max_sectors_per_xfer(); - - let mut out = Vec::::with_capacity((sectors as usize) * sector_size); - let mut remaining = sectors; - let mut lba = start_lba; - let attempts_total = cfg.max_attempts.max(1); - let min_chunk = cfg.min_sectors_per_read.max(1); - - while remaining > 0 { - let mut chunk_sectors = remaining.min(max_sectors_per_xfer); - let mut backoff_ms = cfg.initial_backoff_ms; - let mut last_err: Option = None; - - for attempt in 1..=attempts_total { - match read_cd_chunk(lba, chunk_sectors, mode) { - Ok(chunk) => { - out.extend_from_slice(&chunk); - lba += chunk_sectors; - remaining -= chunk_sectors; - last_err = None; - break; - } - Err(err) => { - last_err = Some(err); - if attempt == attempts_total { - break; - } - if cfg.reduce_chunk_on_retry && chunk_sectors > min_chunk { - chunk_sectors = next_chunk_size(chunk_sectors, min_chunk); - } - if backoff_ms > 0 { - sleep(Duration::from_millis(backoff_ms)); - } - if cfg.max_backoff_ms > 0 { - backoff_ms = (backoff_ms.saturating_mul(2)).min(cfg.max_backoff_ms); - } - } - } - } - - if let Some(err) = last_err { - return Err(err); - } - } - - Ok(out) + crate::read_loop::read_sectors_chunked(start_lba, sectors, mode, cfg, |lba, chunk_sectors| { + read_cd_chunk(lba, chunk_sectors, mode) + }) } fn read_cd_chunk(lba: u32, sectors: u32, mode: &SectorReadMode) -> Result, CdReaderError> { @@ -207,24 +153,16 @@ fn read_cd_chunk(lba: u32, sectors: u32, mode: &SectorReadMode) -> Result unsafe { - read_cd_audio(lba, sectors, &mut buf, &mut len, &mut err) - }, - _ => unsafe { - read_cd_data( - lba, - sectors, - mode.cdb_byte1(), - mode.cdb_byte9(), - mode.sector_size() as u32, - &mut buf, - &mut len, - &mut err, - ) - }, + // Discriminant understood by `read_cd_sectors`, which maps it to the + // matching macOS CD sector area/type for DKIOCCDREAD. + let mode_id: u32 = match mode { + SectorReadMode::Audio => 0, + SectorReadMode::DataCooked => 1, + SectorReadMode::DataRaw => 2, }; + let ok = unsafe { read_cd_sectors(lba, sectors, mode_id, &mut buf, &mut len, &mut err) }; + if !ok { return Err(map_mac_error(err, ScsiOp::ReadCd, Some(lba), Some(sectors))); } @@ -256,11 +194,3 @@ fn map_mac_error( CdReaderError::Io(std::io::Error::other("macOS CD command failed")) } - -fn next_chunk_size(current: u32, min_chunk: u32) -> u32 { - if current > 8 { - 8.max(min_chunk) - } else { - min_chunk - } -} diff --git a/src/read_loop.rs b/src/read_loop.rs new file mode 100644 index 0000000..5df91ac --- /dev/null +++ b/src/read_loop.rs @@ -0,0 +1,88 @@ +//! Shared retry + chunking loop for `READ CD` reads. +//! +//! Every platform issues the same logical read: split a sector range into +//! drive-safe chunks, read each chunk with capped exponential backoff and +//! adaptive chunk-size reduction, and concatenate the results. Only the +//! single-command read itself (SG_IO on Linux, SPTI on Windows, IOKit on +//! macOS) is platform-specific, so it is injected as a closure. + +use std::thread::sleep; +use std::time::Duration; + +use crate::data_reader::SectorReadMode; +use crate::{CdReaderError, RetryConfig}; + +/// Read `sectors` sectors starting at `start_lba` in the given `mode`. +/// +/// `read_chunk(lba, sectors)` performs one platform-specific `READ CD` +/// command and returns the raw bytes for that chunk. The loop owns chunk +/// sizing, retries, and backoff so platform code only implements the +/// single-command read. +pub(crate) fn read_sectors_chunked( + start_lba: u32, + sectors: u32, + mode: &SectorReadMode, + cfg: &RetryConfig, + mut read_chunk: F, +) -> Result, CdReaderError> +where + F: FnMut(u32, u32) -> Result, CdReaderError>, +{ + let total_bytes = (sectors as usize) * mode.sector_size(); + let max_sectors_per_xfer = mode.max_sectors_per_xfer(); + let mut out = Vec::::with_capacity(total_bytes); + + let mut remaining = sectors; + let mut lba = start_lba; + let attempts_total = cfg.max_attempts.max(1); + let min_chunk = cfg.min_sectors_per_read.max(1); + + while remaining > 0 { + let mut chunk_sectors = remaining.min(max_sectors_per_xfer); + let mut backoff_ms = cfg.initial_backoff_ms; + let mut last_err: Option = None; + + for attempt in 1..=attempts_total { + match read_chunk(lba, chunk_sectors) { + Ok(chunk) => { + out.extend_from_slice(&chunk); + lba += chunk_sectors; + remaining -= chunk_sectors; + last_err = None; + break; + } + Err(err) => { + last_err = Some(err); + if attempt == attempts_total { + break; + } + if cfg.reduce_chunk_on_retry && chunk_sectors > min_chunk { + chunk_sectors = next_chunk_size(chunk_sectors, min_chunk); + } + if backoff_ms > 0 { + sleep(Duration::from_millis(backoff_ms)); + } + if cfg.max_backoff_ms > 0 { + backoff_ms = (backoff_ms.saturating_mul(2)).min(cfg.max_backoff_ms); + } + } + } + } + + if let Some(err) = last_err { + return Err(err); + } + } + + Ok(out) +} + +/// Shrink the chunk size after a failed read to improve the odds of success, +/// stepping large reads down toward `min_chunk` (for example `27 -> 8 -> 1`). +fn next_chunk_size(current: u32, min_chunk: u32) -> u32 { + if current > 8 { + 8.max(min_chunk) + } else { + min_chunk + } +} diff --git a/src/windows_read_track.rs b/src/windows_read_track.rs index 1ae4606..b517fa8 100644 --- a/src/windows_read_track.rs +++ b/src/windows_read_track.rs @@ -1,8 +1,5 @@ -use std::cmp::min; use std::mem; use std::ptr; -use std::thread::sleep; -use std::time::Duration; use windows_sys::Win32::Foundation::HANDLE; use windows_sys::Win32::Storage::IscsiDisc::{ @@ -21,54 +18,9 @@ pub fn read_range_with_retry( mode: &SectorReadMode, cfg: &RetryConfig, ) -> Result, CdReaderError> { - let sector_size = mode.sector_size(); - let max_sectors_per_xfer = mode.max_sectors_per_xfer(); - - let total_bytes = (sectors as usize) * sector_size; - let mut out = Vec::::with_capacity(total_bytes); - - let mut remaining = sectors; - let mut lba = start_lba; - let attempts_total = cfg.max_attempts.max(1); - - while remaining > 0 { - let mut chunk_sectors = min(remaining, max_sectors_per_xfer); - let min_chunk = cfg.min_sectors_per_read.max(1); - let mut backoff_ms = cfg.initial_backoff_ms; - let mut last_err: Option = None; - - for attempt in 1..=attempts_total { - match read_cd_chunk(handle, lba, chunk_sectors, mode) { - Ok(chunk) => { - out.extend_from_slice(&chunk); - lba += chunk_sectors; - remaining -= chunk_sectors; - last_err = None; - break; - } - Err(err) => { - last_err = Some(err); - if attempt == attempts_total { - break; - } - if cfg.reduce_chunk_on_retry && chunk_sectors > min_chunk { - chunk_sectors = next_chunk_size(chunk_sectors, min_chunk); - } - if backoff_ms > 0 { - sleep(Duration::from_millis(backoff_ms)); - } - if cfg.max_backoff_ms > 0 { - backoff_ms = (backoff_ms.saturating_mul(2)).min(cfg.max_backoff_ms); - } - } - } - } - if let Some(err) = last_err { - return Err(err); - } - } - - Ok(out) + crate::read_loop::read_sectors_chunked(start_lba, sectors, mode, cfg, |lba, chunk_sectors| { + read_cd_chunk(handle, lba, chunk_sectors, mode) + }) } fn read_cd_chunk( @@ -145,11 +97,3 @@ fn parse_sense(sense: &[u8], sense_len: u8) -> (Option, Option, Option u32 { - if current > 8 { - 8.max(min_chunk) - } else { - min_chunk - } -} From 33fb7df91ea456cd6f98dc17cb7d4f6ef82575f3 Mon Sep 17 00:00:00 2001 From: Dani Sarfati Date: Thu, 28 May 2026 20:26:09 -0400 Subject: [PATCH 7/7] Remove unused get_drive_handle to fix Windows -D warnings build failure Windows twin of the earlier get_drive_fd removal: read_toc and read_sectors_with_mode access DRIVE_HANDLE directly, so the helper was dead code and tripped the dead_code lint under CI's -D warnings. Also addresses the maintainer's review comment on windows.rs:82. Co-Authored-By: Claude Opus 4.8 --- src/windows.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/windows.rs b/src/windows.rs index 1f1fb5e..c952459 100644 --- a/src/windows.rs +++ b/src/windows.rs @@ -79,17 +79,6 @@ pub fn open_drive(path: &str) -> std::io::Result<()> { Ok(()) } -pub(crate) fn get_drive_handle() -> Result { - unsafe { - DRIVE_HANDLE.ok_or_else(|| { - CdReaderError::Io(std::io::Error::new( - std::io::ErrorKind::NotFound, - "Drive not opened", - )) - }) - } -} - pub fn close_drive() { unsafe { if let Some(current_drive) = DRIVE_HANDLE {