[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