diff --git a/src/device.rs b/src/device.rs index 60c162ba..16e906ac 100644 --- a/src/device.rs +++ b/src/device.rs @@ -10,7 +10,7 @@ use crate::{ }, DeviceInfo, Error, MaybeFuture, Speed, }; -use log::error; +use log::{error, warn}; use std::{ future::{poll_fn, Future}, io::ErrorKind, @@ -635,10 +635,26 @@ impl Endpoint { /// For an OUT transfer, the buffer's `len` is the number of bytes /// initialized, which will be sent to the device. /// - /// For an IN transfer, the buffer's `requested_len` is the number of - /// bytes requested. It must be a multiple of the endpoint's [maximum packet - /// size][`Self::max_packet_size`]. + /// For an IN transfer, the buffer's `requested_len` is the number of bytes + /// requested. It must be a nonzero multiple of the endpoint's [maximum + /// packet size][`Self::max_packet_size`] or the transfer will fail with + /// `TransferError::InvalidArgument`. Up to `requested_len / + /// max_packet_size` packets will be received, ending early when any packet + /// is shorter than `max_packet_size`. pub fn submit(&mut self, buf: Buffer) { + if Dir::DIR == Direction::In { + let req_len = buf.requested_len(); + if req_len == 0 || req_len % self.max_packet_size() != 0 { + warn!( + "Submitting transfer with length {req_len} which is not a multiple of max packet size {} on IN endpoint {:02x}", + self.max_packet_size(), + self.endpoint_address(), + ); + + return self.backend.submit_err(buf, TransferError::InvalidArgument); + } + } + self.backend.submit(buf) } @@ -647,6 +663,12 @@ impl Endpoint { /// This future is cancel-safe: it can be cancelled and re-created without /// side effects, enabling its use in `select!{}` or similar. /// + /// An OUT transfer completes when the specified data has been sent or an + /// error occurs. An IN transfer completes when a packet smaller than + /// `max_packet_size` is received, the full `requested_len` is received + /// (without waiting for or consuming any subsequent zero-length packet), or + /// an error occurs. + /// /// ## Panics /// * if there are no transfers pending (that is, if [`Self::pending()`] /// would return 0). diff --git a/src/platform/linux_usbfs/device.rs b/src/platform/linux_usbfs/device.rs index cb47a8d0..922816a7 100644 --- a/src/platform/linux_usbfs/device.rs +++ b/src/platform/linux_usbfs/device.rs @@ -691,18 +691,30 @@ impl LinuxEndpoint { } } - pub(crate) fn submit(&mut self, data: Buffer) { - let mut transfer = self.idle_transfer.take().unwrap_or_else(|| { + fn get_transfer(&mut self) -> Idle { + self.idle_transfer.take().unwrap_or_else(|| { Idle::new( self.inner.clone(), super::TransferData::new(self.inner.address, self.inner.ep_type), ) - }); + }) + } + + pub(crate) fn submit(&mut self, data: Buffer) { + let mut transfer = self.get_transfer(); transfer.set_buffer(data); self.pending .push_back(self.inner.interface.device.submit(transfer)); } + pub(crate) fn submit_err(&mut self, data: Buffer, error: TransferError) { + assert_eq!(error, TransferError::InvalidArgument); + let mut transfer = self.get_transfer(); + transfer.set_buffer(data); + transfer.urb_mut().status = Errno::INVAL.raw_os_error(); + self.pending.push_back(transfer.simulate_complete()); + } + pub(crate) fn poll_next_complete(&mut self, cx: &mut Context) -> Poll { self.inner.notify.subscribe(cx); if let Some(mut transfer) = take_completed_from_queue(&mut self.pending) { diff --git a/src/platform/linux_usbfs/mod.rs b/src/platform/linux_usbfs/mod.rs index 18e17f5b..917edf63 100644 --- a/src/platform/linux_usbfs/mod.rs +++ b/src/platform/linux_usbfs/mod.rs @@ -31,6 +31,7 @@ fn errno_to_transfer_error(e: Errno) -> TransferError { Errno::PROTO | Errno::ILSEQ | Errno::OVERFLOW | Errno::COMM | Errno::TIME => { TransferError::Fault } - _ => TransferError::Unknown(e.raw_os_error()), + Errno::INVAL => TransferError::InvalidArgument, + _ => TransferError::Unknown(e.raw_os_error() as u32), } } diff --git a/src/platform/macos_iokit/device.rs b/src/platform/macos_iokit/device.rs index bd013cd4..f8444d3d 100644 --- a/src/platform/macos_iokit/device.rs +++ b/src/platform/macos_iokit/device.rs @@ -466,25 +466,30 @@ impl MacEndpoint { ); } - pub(crate) fn submit(&mut self, buffer: Buffer) { + fn make_transfer(&mut self, buffer: Buffer) -> Idle { let mut transfer = self .idle_transfer .take() .unwrap_or_else(|| Idle::new(self.inner.clone(), super::TransferData::new())); let buffer = ManuallyDrop::new(buffer); - let endpoint = self.inner.address; - let dir = Direction::from_address(endpoint); - let pipe_ref = self.inner.pipe_ref; - transfer.buf = buffer.ptr; transfer.capacity = buffer.capacity; transfer.actual_len = 0; - let req_len = match dir { + let req_len = match Direction::from_address(self.inner.address) { Direction::Out => buffer.len, Direction::In => buffer.requested_len, }; transfer.requested_len = req_len; + transfer + } + + pub(crate) fn submit(&mut self, buffer: Buffer) { + let transfer = self.make_transfer(buffer); + let endpoint = self.inner.address; + let dir = Direction::from_address(endpoint); + let req_len = transfer.requested_len; + let buf_ptr = transfer.buf; let transfer = transfer.pre_submit(); let ptr = transfer.as_ptr(); @@ -494,9 +499,9 @@ impl MacEndpoint { Direction::Out => call_iokit_function!( self.inner.interface.interface.raw, WritePipeAsync( - pipe_ref, - buffer.ptr as *mut c_void, - buffer.len, + self.inner.pipe_ref, + buf_ptr as *mut c_void, + req_len, transfer_callback, ptr as *mut c_void ) @@ -504,9 +509,9 @@ impl MacEndpoint { Direction::In => call_iokit_function!( self.inner.interface.interface.raw, ReadPipeAsync( - pipe_ref, - buffer.ptr as *mut c_void, - buffer.requested_len, + self.inner.pipe_ref, + buf_ptr as *mut c_void, + req_len, transfer_callback, ptr as *mut c_void ) @@ -519,7 +524,7 @@ impl MacEndpoint { "Submitted {dir:?} transfer {ptr:?} of len {req_len} on endpoint {endpoint:02X}" ); } else { - error!("Failed to submit transfer {ptr:?} of len {req_len} on endpoint {endpoint:02X}: {res:x}"); + error!("Failed to submit {dir:?} transfer {ptr:?} of len {req_len} on endpoint {endpoint:02X}: {res:x}"); unsafe { // Complete the transfer in the place of the callback (*ptr).status = res; @@ -530,6 +535,13 @@ impl MacEndpoint { self.pending.push_back(transfer); } + pub(crate) fn submit_err(&mut self, buffer: Buffer, err: TransferError) { + assert_eq!(err, TransferError::InvalidArgument); + let mut transfer = self.make_transfer(buffer); + transfer.status = io_kit_sys::ret::kIOReturnBadArgument; + self.pending.push_back(transfer.simulate_complete()); + } + pub(crate) fn poll_next_complete(&mut self, cx: &mut Context) -> Poll { self.inner.notify.subscribe(cx); if let Some(mut transfer) = take_completed_from_queue(&mut self.pending) { diff --git a/src/platform/macos_iokit/iokit_c.rs b/src/platform/macos_iokit/iokit_c.rs index 96747b03..dd0bb040 100644 --- a/src/platform/macos_iokit/iokit_c.rs +++ b/src/platform/macos_iokit/iokit_c.rs @@ -61,8 +61,6 @@ pub(crate) const kIOUSBTransactionTimeout: c_int = SYS_IOKIT | SUB_IOKIT_USB | 0 pub(crate) const kIOUSBFindInterfaceDontCare: UInt16 = 0xFFFF; -// - // // Type aliases. // diff --git a/src/platform/macos_iokit/mod.rs b/src/platform/macos_iokit/mod.rs index 3c9bae58..4cc46fcd 100644 --- a/src/platform/macos_iokit/mod.rs +++ b/src/platform/macos_iokit/mod.rs @@ -29,8 +29,11 @@ fn status_to_transfer_result(status: IOReturn) -> Result<(), TransferError> { match status { io_kit_sys::ret::kIOReturnSuccess | io_kit_sys::ret::kIOReturnUnderrun => Ok(()), io_kit_sys::ret::kIOReturnNoDevice => Err(TransferError::Disconnected), - io_kit_sys::ret::kIOReturnAborted => Err(TransferError::Cancelled), + io_kit_sys::ret::kIOReturnAborted | iokit_c::kIOUSBTransactionTimeout => { + Err(TransferError::Cancelled) + } iokit_c::kIOUSBPipeStalled => Err(TransferError::Stall), - _ => Err(TransferError::Unknown(status as i32)), + io_kit_sys::ret::kIOReturnBadArgument => Err(TransferError::InvalidArgument), // used for `submit_err` + _ => Err(TransferError::Unknown(status as u32)), } } diff --git a/src/platform/windows_winusb/device.rs b/src/platform/windows_winusb/device.rs index d00256ae..ef4309aa 100644 --- a/src/platform/windows_winusb/device.rs +++ b/src/platform/windows_winusb/device.rs @@ -21,7 +21,10 @@ use windows_sys::Win32::{ WinUsb_SetPipePolicy, WinUsb_WritePipe, USB_DEVICE_DESCRIPTOR, WINUSB_INTERFACE_HANDLE, WINUSB_SETUP_PACKET, }, - Foundation::{GetLastError, ERROR_IO_PENDING, ERROR_NOT_FOUND, FALSE, HANDLE, TRUE}, + Foundation::{ + GetLastError, ERROR_BAD_COMMAND, ERROR_DEVICE_NOT_CONNECTED, ERROR_FILE_NOT_FOUND, + ERROR_IO_PENDING, ERROR_NOT_FOUND, ERROR_NO_SUCH_DEVICE, FALSE, HANDLE, TRUE, + }, System::IO::{CancelIoEx, OVERLAPPED}, }; @@ -409,10 +412,6 @@ impl WindowsInterface { data: ControlIn, timeout: Duration, ) -> impl MaybeFuture, TransferError>> { - if data.recipient == Recipient::Interface && data.index as u8 != self.interface_number { - warn!("WinUSB sends interface number instead of passed `index` when performing a control transfer with `Recipient::Interface`"); - } - let mut t = TransferData::new(0x80); t.set_buffer(Buffer::new(data.length as usize)); @@ -438,10 +437,6 @@ impl WindowsInterface { data: ControlOut, timeout: Duration, ) -> impl MaybeFuture> { - if data.recipient == Recipient::Interface && data.index as u8 != self.interface_number { - warn!("WinUSB sends interface number instead of passed `index` when performing a control transfer with `Recipient::Interface`"); - } - let mut t = TransferData::new(0x00); t.set_buffer(Buffer::from(data.data.to_vec())); @@ -540,6 +535,7 @@ impl WindowsInterface { let len = t.request_len; let buf = t.buf; t.overlapped.InternalHigh = 0; + t.error_from_submit = Ok(()); let t = t.pre_submit(); let ptr = t.as_ptr(); @@ -580,6 +576,15 @@ impl WindowsInterface { let len = t.request_len; let buf = t.buf; t.overlapped.InternalHigh = 0; + t.error_from_submit = Ok(()); + + if pkt.RequestType & 0x1f == Recipient::Interface as u8 + && pkt.Index as u8 != self.interface_number + { + warn!("WinUSB requires control transfer with `Recipient::Interface` to pass the interface number in `index`"); + t.error_from_submit = Err(TransferError::InvalidArgument); + return t.simulate_complete(); + } let t = t.pre_submit(); let ptr = t.as_ptr(); @@ -613,7 +618,13 @@ impl WindowsInterface { // Safety: Transfer was not submitted, so we still own it // and must complete it in place of the event thread. unsafe { - (*t.as_ptr()).overlapped.Internal = err as _; + (*t.as_ptr()).error_from_submit = match err { + ERROR_BAD_COMMAND + | ERROR_FILE_NOT_FOUND + | ERROR_DEVICE_NOT_CONNECTED + | ERROR_NO_SUCH_DEVICE => Err(TransferError::Disconnected), + other => Err(TransferError::Unknown(other)), + }; notify_completion::(t.as_ptr()); } } @@ -672,15 +683,26 @@ impl WindowsEndpoint { } } - pub(crate) fn submit(&mut self, buffer: Buffer) { + fn make_transfer(&mut self, buffer: Buffer) -> Idle { let mut t = self.idle_transfer.take().unwrap_or_else(|| { Idle::new(self.inner.clone(), TransferData::new(self.inner.address)) }); t.set_buffer(buffer); + t + } + + pub(crate) fn submit(&mut self, buffer: Buffer) { + let t = self.make_transfer(buffer); let t = self.inner.interface.submit(t); self.pending.push_back(t); } + pub(crate) fn submit_err(&mut self, buffer: Buffer, err: TransferError) { + let mut t = self.make_transfer(buffer); + t.error_from_submit = Err(err); + self.pending.push_back(t.simulate_complete()); + } + pub(crate) fn poll_next_complete(&mut self, cx: &mut Context) -> Poll { self.inner.notify.subscribe(cx); if let Some(mut transfer) = take_completed_from_queue(&mut self.pending) { diff --git a/src/platform/windows_winusb/transfer.rs b/src/platform/windows_winusb/transfer.rs index b1f9ccf2..e63b3256 100644 --- a/src/platform/windows_winusb/transfer.rs +++ b/src/platform/windows_winusb/transfer.rs @@ -24,6 +24,7 @@ pub struct TransferData { pub(crate) capacity: u32, pub(crate) request_len: u32, pub(crate) endpoint: u8, + pub(crate) error_from_submit: Result<(), TransferError>, } unsafe impl Send for TransferData {} @@ -39,6 +40,7 @@ impl TransferData { capacity: 0, request_len: 0, endpoint, + error_from_submit: Ok(()), } } @@ -62,19 +64,22 @@ impl TransferData { pub fn take_completion(&mut self, intf: &Interface) -> Completion { let mut actual_len: u32 = 0; - unsafe { GetOverlappedResult(intf.handle, &self.overlapped, &mut actual_len, 0) }; - - let status = match unsafe { GetLastError() } { - ERROR_SUCCESS => Ok(()), - ERROR_GEN_FAILURE => Err(TransferError::Stall), - ERROR_REQUEST_ABORTED | ERROR_TIMEOUT | ERROR_SEM_TIMEOUT | ERROR_OPERATION_ABORTED => { - Err(TransferError::Cancelled) - } - ERROR_FILE_NOT_FOUND | ERROR_DEVICE_NOT_CONNECTED | ERROR_NO_SUCH_DEVICE => { - Err(TransferError::Disconnected) + let status = self.error_from_submit.and_then(|()| { + unsafe { GetOverlappedResult(intf.handle, &self.overlapped, &mut actual_len, 0) }; + + match unsafe { GetLastError() } { + ERROR_SUCCESS => Ok(()), + ERROR_GEN_FAILURE => Err(TransferError::Stall), + ERROR_REQUEST_ABORTED + | ERROR_TIMEOUT + | ERROR_SEM_TIMEOUT + | ERROR_OPERATION_ABORTED => Err(TransferError::Cancelled), + ERROR_FILE_NOT_FOUND | ERROR_DEVICE_NOT_CONNECTED | ERROR_NO_SUCH_DEVICE => { + Err(TransferError::Disconnected) + } + e => Err(TransferError::Unknown(e)), } - e => Err(TransferError::Unknown(e as i32)), - }; + }); let mut empty = ManuallyDrop::new(Vec::new()); let ptr = mem::replace(&mut self.buf, empty.as_mut_ptr()); diff --git a/src/transfer/internal.rs b/src/transfer/internal.rs index cae81d04..21298b2e 100644 --- a/src/transfer/internal.rs +++ b/src/transfer/internal.rs @@ -128,6 +128,12 @@ impl

Idle

{ ptr: unsafe { NonNull::new_unchecked(Box::into_raw(self.0)) }, } } + + pub(crate) fn simulate_complete(self) -> Pending

{ + Pending { + ptr: unsafe { NonNull::new_unchecked(Box::into_raw(self.0)) }, + } + } } impl

Deref for Idle

{ diff --git a/src/transfer/mod.rs b/src/transfer/mod.rs index 05794f8b..2d99b863 100644 --- a/src/transfer/mod.rs +++ b/src/transfer/mod.rs @@ -39,13 +39,16 @@ pub enum TransferError { /// Hardware issue or protocol violation. Fault, + /// The request has an invalid argument or is not supported by this OS. + InvalidArgument, + /// Unknown or OS-specific error. /// /// It won't be considered a breaking change to map unhandled errors from /// `Unknown` to one of the above variants. If you are matching on the /// OS-specific code because an error is not correctly mapped, please open /// an issue or pull request. - Unknown(i32), + Unknown(u32), } impl Display for TransferError { @@ -55,8 +58,13 @@ impl Display for TransferError { TransferError::Stall => write!(f, "endpoint stalled"), TransferError::Disconnected => write!(f, "device disconnected"), TransferError::Fault => write!(f, "hardware fault or protocol violation"), + TransferError::InvalidArgument => write!(f, "invalid or unsupported argument"), TransferError::Unknown(e) => { - write!(f, "unknown error ({})", io::Error::from_raw_os_error(*e)) + if cfg!(target_os = "macos") { + write!(f, "unknown error (0x{e:08x})") + } else { + write!(f, "unknown error ({e})") + } } } } @@ -71,6 +79,7 @@ impl From for io::Error { TransferError::Stall => io::Error::new(io::ErrorKind::ConnectionReset, value), TransferError::Disconnected => io::Error::new(io::ErrorKind::ConnectionAborted, value), TransferError::Fault => io::Error::other(value), + TransferError::InvalidArgument => io::Error::new(io::ErrorKind::InvalidInput, value), TransferError::Unknown(_) => io::Error::other(value), } }