[PATCH] linux: Don't report LIBUSB_ERROR_NOT_FOUND when cancelling unreaped urbs
Hans de Goede
hdegoede at redhat.com
Fri Feb 10 11:54:59 EST 2012
IOCTL_USBFS_DISCARDURB sets errno to EINVAL when the kernel could not find the
urb in the kernels urbs in flight list. Which means that the urb has already
completed. However:
1) The only caller of discard_urbs who checks the return value is
op_cancel_transfer
2) op_cancel_transfer checks that tpriv->urbs != NULL before calling
discard_urbs
3) Since tpriv->urbs != NULL we know that:
a) The urb has been submitted
b) The urb has not been reaped yet
4) Since op_cancel_transfer sets tpriv->reap_action to CANCELLED before
calling discard_urbs, when the urb eventually does get reaped its callback
will get called with a status of LIBUSB_TRANSFER_CANCELLED, even if
the IOCTL_USBFS_DISCARDURB call failed with an errno of EINVAL
Thus I believe it is wrong for op_cancel_transfer to return
LIBUSB_ERROR_NOT_FOUND in the case when the urb has completed at the kernel
level, but has not yet been reaped esp. since there is no race free way for an
app to deal with this, even if an app calls libusb_handle_events immediately
before calling libusb_cancel_transfer, the urb may still complete in between.
Moreover in this failure scenario:
1) libusb_cancel_transfer returns LIBUSB_ERROR_NOT_FOUND, which one would
assume means that the transfers callback function won't get called, since
the transfer was appearently not in flight; but
2) the transfer callback will still get called as soon as the urb gets
reaped (from libusb_handle_events*); *and* not only will it get called, but
3) the transfer callback will report a status of LIBUSB_TRANSFER_CANCELLED
despite libusb_cancel_transfer reporting a failure to cancel!
Given all of the above this patch modifies discard_urbs to treat
an errno of EINVAL as success, since this simply means that an *unreaped*
urb has already completed, which is an unavoidable scenario and as such
should not be treated as an error.
Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
libusb/os/linux_usbfs.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
index 2b81189..bd9caa3 100644
--- a/libusb/os/linux_usbfs.c
+++ b/libusb/os/linux_usbfs.c
@@ -1466,7 +1466,6 @@ static int discard_urbs(struct usbi_transfer *itransfer, int first, int last_plu
if (EINVAL == errno) {
usbi_dbg("URB not found --> assuming ready to be reaped");
- ret = LIBUSB_ERROR_NOT_FOUND;
} else if (ENODEV == errno) {
usbi_dbg("Device not found for URB --> assuming ready to be reaped");
ret = LIBUSB_ERROR_NO_DEVICE;
--
1.7.7.6
More information about the libusbx
mailing list