[libusb1] Add some small error handling fixes
Hans de Goede
jwrdegoede at fedoraproject.org
Wed Mar 14 15:40:39 UTC 2012
commit cd757587c687cb8dce36f27c683d93cd9c9820d8
Author: Hans de Goede <hdegoede at redhat.com>
Date: Wed Mar 14 16:42:31 2012 +0100
Add some small error handling fixes
...ancel_transfer-return-value-when-cancelli.patch | 37 ++++++++
...-errors-when-cancel_transfer-fails-with-N.patch | 46 ++++++++++
0003-linux-Fix-handling-of-urb-status-codes.patch | 92 ++++++++++++++++++++
...late-linux-iso-pkt-status-codes-to-libusb.patch | 68 ++++++++++++++
libusb1.spec | 15 +++-
5 files changed, 257 insertions(+), 1 deletions(-)
---
diff --git a/0001-linux-Fix-cancel_transfer-return-value-when-cancelli.patch b/0001-linux-Fix-cancel_transfer-return-value-when-cancelli.patch
new file mode 100644
index 0000000..91be253
--- /dev/null
+++ b/0001-linux-Fix-cancel_transfer-return-value-when-cancelli.patch
@@ -0,0 +1,37 @@
+From 52508a86e26f0bc74b0a7a3b05ed08a29996b44c Mon Sep 17 00:00:00 2001
+From: Hans de Goede <hdegoede at redhat.com>
+Date: Mon, 20 Feb 2012 16:05:48 +0100
+Subject: [PATCH 1/6] linux: Fix cancel_transfer return value when cancelling
+ a multi-urb transfer
+
+If we fail to cancel the last urb of a multi-urb transfer because it
+has already completed (errno == EINVAL on DISCARD_URB), then the entire
+transfer has already completed, so returning NOT_FOUND is consistent with what
+the documentation for libusb_cancel_transfer says.
+
+But if we've successfully cancelled the last urb, and then another urb
+fails with errno == EINVAL, this means that we've still cancelled the
+transfer, as it has only *partially* completed.
+
+Signed-off-by: Hans de Goede <hdegoede at redhat.com>
+---
+ libusb/os/linux_usbfs.c | 3 ++-
+ 1 file changed, 2 insertions(+), 1 deletion(-)
+
+diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
+index 2b81189..099fc61 100644
+--- a/libusb/os/linux_usbfs.c
++++ b/libusb/os/linux_usbfs.c
+@@ -1466,7 +1466,8 @@ 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;
++ if (i == (last_plus_one - 1))
++ 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.9.3
+
diff --git a/0002-Don-t-print-errors-when-cancel_transfer-fails-with-N.patch b/0002-Don-t-print-errors-when-cancel_transfer-fails-with-N.patch
new file mode 100644
index 0000000..ccb76a1
--- /dev/null
+++ b/0002-Don-t-print-errors-when-cancel_transfer-fails-with-N.patch
@@ -0,0 +1,46 @@
+From e8c0b72bf8cc6d89c3546bbdbcc85b2c63086578 Mon Sep 17 00:00:00 2001
+From: Hans de Goede <hdegoede at redhat.com>
+Date: Mon, 20 Feb 2012 16:12:19 +0100
+Subject: [PATCH 2/6] Don't print errors when cancel_transfer fails with
+ NOT_FOUND
+
+As stated in the documentation for libusb_cancel_transfer,
+LIBUSB_ERROR_NOT_FOUND is an expected return value for
+libusb_cancel_transfer (under certain circumstances) printing
+an error each time this happens therefor is undesirable.
+
+More so because under Linux 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
+at the host controller level, but it has not necessarily already
+been reaped. IOW under Linux libusb_cancel_transfer may yield a
+result of LIBUSB_ERROR_NOT_FOUND *before* the transfer's callback
+has been called! So there is no way for an application to avoid
+calling libusb_cancel_transfer on already completed transfers.
+
+Signed-off-by: Hans de Goede <hdegoede at redhat.com>
+---
+ libusb/io.c | 7 +++++--
+ 1 file changed, 5 insertions(+), 2 deletions(-)
+
+diff --git a/libusb/io.c b/libusb/io.c
+index bb6e275..9f46cf0 100644
+--- a/libusb/io.c
++++ b/libusb/io.c
+@@ -1351,8 +1351,11 @@ int API_EXPORTED libusb_cancel_transfer(struct libusb_transfer *transfer)
+ usbi_mutex_lock(&itransfer->lock);
+ r = usbi_backend->cancel_transfer(itransfer);
+ if (r < 0) {
+- usbi_err(TRANSFER_CTX(transfer),
+- "cancel transfer failed error %d", r);
++ if (r != LIBUSB_ERROR_NOT_FOUND)
++ usbi_err(TRANSFER_CTX(transfer),
++ "cancel transfer failed error %d", r);
++ else
++ usbi_dbg("cancel transfer failed error %d", r);
+
+ if (r == LIBUSB_ERROR_NO_DEVICE)
+ itransfer->flags |= USBI_TRANSFER_DEVICE_DISAPPEARED;
+--
+1.7.9.3
+
diff --git a/0003-linux-Fix-handling-of-urb-status-codes.patch b/0003-linux-Fix-handling-of-urb-status-codes.patch
new file mode 100644
index 0000000..75ef04e
--- /dev/null
+++ b/0003-linux-Fix-handling-of-urb-status-codes.patch
@@ -0,0 +1,92 @@
+From 3b6ed16cd2f098dd3920853d20940b3560c20ece Mon Sep 17 00:00:00 2001
+From: Hans de Goede <hdegoede at redhat.com>
+Date: Fri, 24 Feb 2012 10:24:00 +0100
+Subject: [PATCH 3/6] linux: Fix handling of urb status codes
+
+During testing of my usbredir code I hit a case where EOVERFLOW was not handled
+in handle_control_completion. Instead of just fixing this one case I've audited
+(and fixed where necessary) all handle_foo_completion functions to know about
+all errors documented in linux/Documentation/usb/error-codes.txt.
+
+Note that for handle_iso_completion this patch actually removes the handling
+of some codes, since these can never occur on an iso urb (they can only
+occur on the iso packets included in the urb, see the next patch in this
+series). Also in case an unknown status is encountered on an iso urb, this
+patch actually sets the urb's status to ERROR, rather then leaving it at
+completed.
+
+Signed-off-by: Hans de Goede <hdegoede at redhat.com>
+---
+ libusb/os/linux_usbfs.c | 17 ++++++++++++-----
+ 1 file changed, 12 insertions(+), 5 deletions(-)
+
+diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
+index 099fc61..36d37a4 100644
+--- a/libusb/os/linux_usbfs.c
++++ b/libusb/os/linux_usbfs.c
+@@ -1952,6 +1952,7 @@ static int handle_bulk_completion(struct usbi_transfer *itransfer,
+ case -ENOENT: /* cancelled */
+ case -ECONNRESET:
+ break;
++ case -ENODEV:
+ case -ESHUTDOWN:
+ usbi_dbg("device removed");
+ tpriv->reap_status = LIBUSB_TRANSFER_NO_DEVICE;
+@@ -1970,6 +1971,8 @@ static int handle_bulk_completion(struct usbi_transfer *itransfer,
+ case -ETIME:
+ case -EPROTO:
+ case -EILSEQ:
++ case -ECOMM:
++ case -ENOSR:
+ usbi_dbg("low level error %d", urb->status);
+ tpriv->reap_action = ERROR;
+ goto cancel_remaining;
+@@ -2081,19 +2084,16 @@ static int handle_iso_completion(struct usbi_transfer *itransfer,
+ case 0:
+ break;
+ case -ENOENT: /* cancelled */
++ case -ECONNRESET:
+ break;
+ case -ESHUTDOWN:
+ usbi_dbg("device removed");
+ status = LIBUSB_TRANSFER_NO_DEVICE;
+ break;
+- case -ETIME:
+- case -EPROTO:
+- case -EILSEQ:
+- usbi_dbg("low-level USB error %d", urb->status);
+- break;
+ default:
+ usbi_warn(TRANSFER_CTX(transfer),
+ "unrecognised urb status %d", urb->status);
++ status = LIBUSB_TRANSFER_ERROR;
+ break;
+ }
+
+@@ -2139,6 +2139,7 @@ static int handle_control_completion(struct usbi_transfer *itransfer,
+ case -ENOENT: /* cancelled */
+ status = LIBUSB_TRANSFER_CANCELLED;
+ break;
++ case -ENODEV:
+ case -ESHUTDOWN:
+ usbi_dbg("device removed");
+ status = LIBUSB_TRANSFER_NO_DEVICE;
+@@ -2147,9 +2148,15 @@ static int handle_control_completion(struct usbi_transfer *itransfer,
+ usbi_dbg("unsupported control request");
+ status = LIBUSB_TRANSFER_STALL;
+ break;
++ case -EOVERFLOW:
++ usbi_dbg("control overflow error");
++ status = LIBUSB_TRANSFER_OVERFLOW;
++ break;
+ case -ETIME:
+ case -EPROTO:
+ case -EILSEQ:
++ case -ECOMM:
++ case -ENOSR:
+ usbi_dbg("low-level bus error occurred");
+ status = LIBUSB_TRANSFER_ERROR;
+ break;
+--
+1.7.9.3
+
diff --git a/0004-linux-Translate-linux-iso-pkt-status-codes-to-libusb.patch b/0004-linux-Translate-linux-iso-pkt-status-codes-to-libusb.patch
new file mode 100644
index 0000000..9323783
--- /dev/null
+++ b/0004-linux-Translate-linux-iso-pkt-status-codes-to-libusb.patch
@@ -0,0 +1,68 @@
+From 4c3e7f9818c0d1d0462fde6f219da65fb102a434 Mon Sep 17 00:00:00 2001
+From: Hans de Goede <hdegoede at redhat.com>
+Date: Fri, 24 Feb 2012 11:15:30 +0100
+Subject: [PATCH 4/6] linux: Translate linux iso pkt status codes to libusb
+ transfer status codes
+
+During testing of my usbredir code I hit a scenario where my libusb app
+was seeing EXDEV as status in the transfer's iso_packet_desc
+
+This happened because we don't translate linux negative errno errors
+stored in iso pkts status to libusb transfer status codes at all! So this
+patch adds translation for this.
+
+Signed-off-by: Hans de Goede <hdegoede at redhat.com>
+---
+ libusb/os/linux_usbfs.c | 36 +++++++++++++++++++++++++++++++++++-
+ 1 file changed, 35 insertions(+), 1 deletion(-)
+
+diff --git a/libusb/os/linux_usbfs.c b/libusb/os/linux_usbfs.c
+index 36d37a4..a7d8298 100644
+--- a/libusb/os/linux_usbfs.c
++++ b/libusb/os/linux_usbfs.c
+@@ -2053,7 +2053,41 @@ static int handle_iso_completion(struct usbi_transfer *itransfer,
+ struct usbfs_iso_packet_desc *urb_desc = &urb->iso_frame_desc[i];
+ struct libusb_iso_packet_descriptor *lib_desc =
+ &transfer->iso_packet_desc[tpriv->iso_packet_offset++];
+- lib_desc->status = urb_desc->status;
++ lib_desc->status = LIBUSB_TRANSFER_COMPLETED;
++ switch (urb_desc->status) {
++ case 0:
++ break;
++ case -ENOENT: /* cancelled */
++ case -ECONNRESET:
++ break;
++ case -ENODEV:
++ case -ESHUTDOWN:
++ usbi_dbg("device removed");
++ lib_desc->status = LIBUSB_TRANSFER_NO_DEVICE;
++ break;
++ case -EPIPE:
++ usbi_dbg("detected endpoint stall");
++ lib_desc->status = LIBUSB_TRANSFER_STALL;
++ break;
++ case -EOVERFLOW:
++ usbi_dbg("overflow error");
++ lib_desc->status = LIBUSB_TRANSFER_OVERFLOW;
++ break;
++ case -ETIME:
++ case -EPROTO:
++ case -EILSEQ:
++ case -ECOMM:
++ case -ENOSR:
++ case -EXDEV:
++ usbi_dbg("low-level USB error %d", urb_desc->status);
++ lib_desc->status = LIBUSB_TRANSFER_ERROR;
++ break;
++ default:
++ usbi_warn(TRANSFER_CTX(transfer),
++ "unrecognised urb status %d", urb_desc->status);
++ lib_desc->status = LIBUSB_TRANSFER_ERROR;
++ break;
++ }
+ lib_desc->actual_length = urb_desc->actual_length;
+ }
+
+--
+1.7.9.3
+
diff --git a/libusb1.spec b/libusb1.spec
index accb77d..bf2e1e8 100644
--- a/libusb1.spec
+++ b/libusb1.spec
@@ -1,7 +1,7 @@
Summary: A library which allows userspace access to USB devices
Name: libusb1
Version: 1.0.9
-Release: 0.4.rc1%{?dist}
+Release: 0.5.rc1%{?dist}
# This is a git snapshot of what will hopefully soon become 1.0.9, but
# we need this now, to get things in place for:
# http://fedoraproject.org/wiki/Features/UsbNetworkRedirection
@@ -14,6 +14,12 @@ Release: 0.4.rc1%{?dist}
# mv libusb-1.0.8.tar.bz2 libusb-1.0.9-rc1.tar.bz2
Source0: libusb-1.0.9-rc1.tar.bz2
#Source0: http://downloads.sourceforge.net/libusb/libusb-%{version}.tar.bz2
+
+Patch1: 0001-linux-Fix-cancel_transfer-return-value-when-cancelli.patch
+Patch2: 0002-Don-t-print-errors-when-cancel_transfer-fails-with-N.patch
+Patch3: 0003-linux-Fix-handling-of-urb-status-codes.patch
+Patch4: 0004-linux-Translate-linux-iso-pkt-status-codes-to-libusb.patch
+
License: LGPLv2+
Group: System Environment/Libraries
URL: http://libusb.wiki.sourceforge.net/Libusb1.0
@@ -54,6 +60,10 @@ This package contains static libraries to develop applications that use libusb1.
%prep
%setup -q -n libusb-1.0.8
+%patch1 -p1
+%patch2 -p1
+%patch3 -p1
+%patch4 -p1
%build
%configure --libdir=/%{_lib}
@@ -97,6 +107,9 @@ mv %{buildroot}/%{_lib}/pkgconfig/* %{buildroot}%{_libdir}/pkgconfig/
/%{_lib}/*.a
%changelog
+* Wed Mar 14 2012 Hans de Goede <hdegoede at redhat.com> - 1.0.9-0.5.rc1
+- Add some small error handling fixes
+
* Fri Jan 13 2012 Fedora Release Engineering <rel-eng at lists.fedoraproject.org> - 1.0.9-0.4.rc1
- Rebuilt for https://fedoraproject.org/wiki/Fedora_17_Mass_Rebuild
More information about the scm-commits
mailing list