diff --git a/improvements.md b/improvements.md index fad14d3..e16a060 100644 --- a/improvements.md +++ b/improvements.md @@ -65,12 +65,12 @@ ### 11. Error status mapping via string matching - **File:** `lib/src/lib.rs:584-592` - **Issue:** Fragile string-based error classification depends on nusb/rusb message text. -- **Status:** [ ] TODO +- **Status:** [x] DONE — Preserve nusb `TransferError` through `io::Error`; classify via `ErrorKind` instead of string matching. ### 12. Poisoned mutex panics - **Files:** `host.rs`, `device.rs` (multiple locations) - **Issue:** All `Mutex::lock().unwrap()` calls panic if the mutex is poisoned. -- **Status:** [ ] TODO +- **Status:** [x] DONE — Replaced 21 production `.lock().unwrap()` with `.lock().unwrap_or_else(|e| e.into_inner())`. ### 13. `debug_assert` for path/bus_id length - **File:** `lib/src/device.rs:252-260` diff --git a/lib/src/cdc.rs b/lib/src/cdc.rs index 091ad17..99915c0 100644 --- a/lib/src/cdc.rs +++ b/lib/src/cdc.rs @@ -75,7 +75,7 @@ impl UsbInterfaceHandler for UsbCdcAcmHandler { } else { // bulk in let max_packet_size = ep.max_packet_size as usize; - let mut tx_buffer = self.tx_buffer.lock().unwrap(); + let mut tx_buffer = self.tx_buffer.lock().unwrap_or_else(|e| e.into_inner()); let resp = if tx_buffer.len() > max_packet_size { tx_buffer.drain(..max_packet_size).collect::>() } else { diff --git a/lib/src/device.rs b/lib/src/device.rs index 4b77aaa..9b0a40c 100644 --- a/lib/src/device.rs +++ b/lib/src/device.rs @@ -420,7 +420,7 @@ impl UsbDevice { debug!("Get BOS descriptor"); if self.device_handler.is_some() { let lock = self.device_handler.as_ref().unwrap(); - let mut handler = lock.lock().unwrap(); + let mut handler = lock.lock().unwrap_or_else(|e| e.into_inner()); return handler.handle_urb(UrbRequest { ep, transfer_buffer_length, @@ -448,7 +448,7 @@ impl UsbDevice { // IADs, class-specific descriptors, etc. are preserved. if self.device_handler.is_some() { let lock = self.device_handler.as_ref().unwrap(); - let mut handler = lock.lock().unwrap(); + let mut handler = lock.lock().unwrap_or_else(|e| e.into_inner()); return handler.handle_urb(UrbRequest { ep, transfer_buffer_length, @@ -554,7 +554,7 @@ impl UsbDevice { // Forward unknown string indices to the device handler // (host passthrough: the real device knows its own strings) let lock = self.device_handler.as_ref().unwrap(); - let mut handler = lock.lock().unwrap(); + let mut handler = lock.lock().unwrap_or_else(|e| e.into_inner()); handler.handle_urb(UrbRequest { ep, transfer_buffer_length, @@ -573,7 +573,7 @@ impl UsbDevice { debug!("Get device qualifier descriptor"); if self.device_handler.is_some() { let lock = self.device_handler.as_ref().unwrap(); - let mut handler = lock.lock().unwrap(); + let mut handler = lock.lock().unwrap_or_else(|e| e.into_inner()); return handler.handle_urb(UrbRequest { ep, transfer_buffer_length, @@ -671,7 +671,7 @@ impl UsbDevice { // to device // see https://www.beyondlogic.org/usbnutshell/usb6.shtml let lock = self.device_handler.as_ref().unwrap(); - let mut handler = lock.lock().unwrap(); + let mut handler = lock.lock().unwrap_or_else(|e| e.into_inner()); handler.handle_urb(UrbRequest { ep, transfer_buffer_length, @@ -700,7 +700,7 @@ impl UsbDevice { // Forward to physical device handler if present, // so endpoints are properly reset on the device if let Some(ref handler_lock) = self.device_handler { - let mut handler = handler_lock.lock().unwrap(); + let mut handler = handler_lock.lock().unwrap_or_else(|e| e.into_inner()); handler.handle_urb(UrbRequest { ep, transfer_buffer_length, @@ -782,7 +782,7 @@ impl UsbDevice { // to device // see https://www.beyondlogic.org/usbnutshell/usb6.shtml let lock = self.device_handler.as_ref().unwrap(); - let mut handler = lock.lock().unwrap(); + let mut handler = lock.lock().unwrap_or_else(|e| e.into_inner()); handler.handle_urb(UrbRequest { ep, transfer_buffer_length, diff --git a/lib/src/hid.rs b/lib/src/hid.rs index f73ca97..a6f345d 100644 --- a/lib/src/hid.rs +++ b/lib/src/hid.rs @@ -133,10 +133,10 @@ impl UsbInterfaceHandler for UsbHidKeyboardHandler { // interrupt transfer if let Direction::In = ep.direction() { // interrupt in - let mut state = self.state.lock().unwrap(); + let mut state = self.state.lock().unwrap_or_else(|e| e.into_inner()); match *state { UsbHidKeyboardHandlerState::Idle => { - if let Some(report) = self.pending_key_events.lock().unwrap().pop_front() { + if let Some(report) = self.pending_key_events.lock().unwrap_or_else(|e| e.into_inner()).pop_front() { let mut resp = vec![report.modifier, 0]; resp.extend_from_slice(&report.keys); info!("HID key down"); diff --git a/lib/src/host.rs b/lib/src/host.rs index f1d0042..7e69447 100644 --- a/lib/src/host.rs +++ b/lib/src/host.rs @@ -42,7 +42,7 @@ impl UsbInterfaceHandler for RusbUsbHostInterfaceHandler { debug!("To host device: ep={ep:?} setup={setup:?} req={req:?}",); let mut buffer = vec![0u8; transfer_buffer_length as usize]; let timeout = std::time::Duration::new(1, 0); - let handle = self.handle.lock().unwrap(); + let handle = self.handle.lock().unwrap_or_else(|e| e.into_inner()); if ep.attributes == EndpointAttributes::Control as u8 { // control if let Direction::In = ep.direction() { @@ -151,7 +151,7 @@ impl UsbDeviceHandler for RusbUsbHostDeviceHandler { debug!("To host device: setup={setup:?} req={req:?}"); let mut buffer = vec![0u8; transfer_buffer_length as usize]; let timeout = std::time::Duration::new(1, 0); - let handle = self.handle.lock().unwrap(); + let handle = self.handle.lock().unwrap_or_else(|e| e.into_inner()); // control if setup.request_type & 0x80 == 0 { // control out @@ -213,10 +213,8 @@ impl NusbUsbHostInterfaceHandler { impl UsbInterfaceHandler for NusbUsbHostInterfaceHandler { fn set_alt_setting(&self, alt: u8) -> Result<()> { - let handle = self.handle.lock().unwrap(); - handle.set_alt_setting(alt).wait().map_err(|e| { - std::io::Error::other(format!("Failed to set alt setting {alt}: {e}")) - }) + let handle = self.handle.lock().unwrap_or_else(|e| e.into_inner()); + handle.set_alt_setting(alt).wait().map_err(Into::into) } fn handle_urb( @@ -237,7 +235,7 @@ impl UsbInterfaceHandler for NusbUsbHostInterfaceHandler { 0x01 => std::time::Duration::from_secs(5), // isochronous _ => std::time::Duration::from_secs(1), // control, bulk }; - let handle = self.handle.lock().unwrap(); + let handle = self.handle.lock().unwrap_or_else(|e| e.into_inner()); if ep.attributes == EndpointAttributes::Control as u8 { // control if let Direction::In = ep.direction() { @@ -271,9 +269,7 @@ impl UsbInterfaceHandler for NusbUsbHostInterfaceHandler { index: setup.index, length: transfer_buffer_length.min(u16::MAX as u32) as u16, }; - let data = handle.control_in(control_in, timeout).wait().map_err(|e| { - std::io::Error::new(std::io::ErrorKind::Other, format!("USB control IN failed: {e}")) - })?; + let data = handle.control_in(control_in, timeout).wait().map_err(std::io::Error::from)?; return Ok(UrbResponse { data, ..Default::default() }); } else { // control out @@ -309,12 +305,7 @@ impl UsbInterfaceHandler for NusbUsbHostInterfaceHandler { handle .control_out(control_out, timeout) .wait() - .map_err(|e| { - std::io::Error::new( - std::io::ErrorKind::Other, - format!("USB control OUT failed: {e}"), - ) - })?; + .map_err(std::io::Error::from)?; } } else if ep.attributes == EndpointAttributes::Interrupt as u8 { // interrupt @@ -340,22 +331,18 @@ impl UsbInterfaceHandler for NusbUsbHostInterfaceHandler { Ok(()) => { return Ok(UrbResponse { data: completion.buffer.to_vec(), ..Default::default() }); } - Err(e) => { - let msg = e.to_string(); - if msg.contains("timed out") || msg.contains("cancel") { - if std::time::Instant::now() >= deadline { - return Err(std::io::Error::new( - std::io::ErrorKind::TimedOut, - format!("USB interrupt IN timed out after {timeout:?}"), - )); - } - // Retry silently - continue; + Err(nusb::transfer::TransferError::Cancelled) => { + if std::time::Instant::now() >= deadline { + return Err(std::io::Error::new( + std::io::ErrorKind::TimedOut, + "USB interrupt IN timed out", + )); } - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - format!("USB interrupt IN failed: {e}"), - )); + // Retry silently + continue; + } + Err(e) => { + return Err(std::io::Error::from(e)); } } } @@ -370,9 +357,7 @@ impl UsbInterfaceHandler for NusbUsbHostInterfaceHandler { buffer.extend_from_slice(req); drop(handle); let completion = endpoint.transfer_blocking(buffer, timeout); - completion.status.map_err(|e| { - std::io::Error::new(std::io::ErrorKind::Other, format!("USB interrupt OUT failed: {e}")) - })?; + completion.status.map_err(std::io::Error::from)?; } } else if ep.attributes == EndpointAttributes::Bulk as u8 { // bulk @@ -387,9 +372,7 @@ impl UsbInterfaceHandler for NusbUsbHostInterfaceHandler { })?; let buffer = endpoint.allocate(alloc_len); let completion = endpoint.transfer_blocking(buffer, timeout); - completion.status.map_err(|e| { - std::io::Error::new(std::io::ErrorKind::Other, format!("USB bulk IN failed: {e}")) - })?; + completion.status.map_err(std::io::Error::from)?; // Return only the actual bytes received (may be less than alloc_len) return Ok(UrbResponse { data: completion.buffer.to_vec(), ..Default::default() }); } else { @@ -402,9 +385,7 @@ impl UsbInterfaceHandler for NusbUsbHostInterfaceHandler { let mut buffer = endpoint.allocate(req.len()); buffer.extend_from_slice(req); let completion = endpoint.transfer_blocking(buffer, timeout); - completion.status.map_err(|e| { - std::io::Error::new(std::io::ErrorKind::Other, format!("USB bulk OUT failed: {e}")) - })?; + completion.status.map_err(std::io::Error::from)?; } } else if ep.attributes == EndpointAttributes::Isochronous as u8 { // Isochronous transfer @@ -545,7 +526,7 @@ impl UsbDeviceHandler for NusbUsbHostDeviceHandler { let req = &request.data; debug!("To host device: setup={setup:?} req={req:?}"); let timeout = std::time::Duration::new(1, 0); - let handle = self.handle.lock().unwrap(); + let handle = self.handle.lock().unwrap_or_else(|e| e.into_inner()); // control if cfg!(not(target_os = "windows")) { if setup.request_type & 0x80 == 0 { @@ -584,12 +565,7 @@ impl UsbDeviceHandler for NusbUsbHostDeviceHandler { handle .control_out(control_out, timeout) .wait() - .map_err(|e| { - std::io::Error::new( - std::io::ErrorKind::Other, - format!("USB control OUT failed: {e}"), - ) - })?; + .map_err(std::io::Error::from)?; } } else { // control in @@ -624,9 +600,7 @@ impl UsbDeviceHandler for NusbUsbHostDeviceHandler { index: setup.index, length: transfer_buffer_length.min(u16::MAX as u32) as u16, }; - let data = handle.control_in(control_in, timeout).wait().map_err(|e| { - std::io::Error::new(std::io::ErrorKind::Other, format!("USB control IN failed: {e}")) - })?; + let data = handle.control_in(control_in, timeout).wait().map_err(std::io::Error::from)?; return Ok(UrbResponse { data, ..Default::default() }); } } diff --git a/lib/src/lib.rs b/lib/src/lib.rs index 4c2fd44..05f3586 100644 --- a/lib/src/lib.rs +++ b/lib/src/lib.rs @@ -489,7 +489,7 @@ pub async fn handle_urb_loop { // Don't time out if there are in-flight URBs (e.g. pending // interrupt transfers) — the kernel is still alive. - if !in_flight.lock().unwrap().is_empty() { + if !in_flight.lock().unwrap_or_else(|e| e.into_inner()).is_empty() { continue; } warn!("URB read timed out after {URB_READ_TIMEOUT:?}"); @@ -556,7 +556,7 @@ pub async fn handle_urb_loop= MAX_IN_FLIGHT_URBS { + if in_flight.lock().unwrap_or_else(|e| e.into_inner()).len() >= MAX_IN_FLIGHT_URBS { warn!( "Too many in-flight URBs ({MAX_IN_FLIGHT_URBS}), rejecting seqnum={seqnum}" ); @@ -588,14 +588,14 @@ pub async fn handle_urb_loop { - let msg = err.to_string(); - // Map USB errors to appropriate Linux URB status codes - let status = if msg.contains("stall") { - (-32i32) as u32 // EPIPE: endpoint stalled - } else if msg.contains("cancel") { - (-2i32) as u32 // ENOENT: URB was unlinked/cancelled - } else if msg.contains("timed out") || msg.contains("TimedOut") { - (-110i32) as u32 // ETIMEDOUT - } else { - (-71i32) as u32 // EPROTO: protocol error + // Map io::ErrorKind to appropriate Linux URB status codes. + // nusb's TransferError → io::Error preserves the kind: + // Stall → ConnectionReset, Cancelled → Interrupted, + // Disconnected → ConnectionAborted + let status = match err.kind() { + std::io::ErrorKind::ConnectionReset => (-32i32) as u32, // EPIPE: stall + std::io::ErrorKind::Interrupted => (-2i32) as u32, // ENOENT: cancelled + std::io::ErrorKind::TimedOut => (-110i32) as u32, // ETIMEDOUT + std::io::ErrorKind::ConnectionAborted => (-108i32) as u32, // ESHUTDOWN: disconnected + _ => (-71i32) as u32, // EPROTO: protocol error }; warn!("Error handling URB seqnum={seqnum}: {err} (status={status})", status = status as i32); UsbIpResponse::UsbIpRetSubmit { @@ -689,11 +689,11 @@ pub async fn handle_urb_loop {