Re: [Qemu-devel] [PATCH 05/14] usb-linux: Don't call usb_host_close when usb_host_open fails

[Gerd Hoffmann]

What bug you are trying to fix here?

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 05/14] usb-linux: Don't call usb_host_close when usb_host_open fails

[Hans de Goede]

Hi,

On 06/01/2011 02:22 PM, Gerd Hoffmann wrote:


What bug you are trying to fix here?


Nothing in particular, while looking at some other
stuff I noticed that we have the following sequence,
which is wrong:

usb_host_open called
 usb_host_open calls usb_host_claim_interfaces
  usb_host_claim_interfaces calls do_disconnect because of failure
   do_disconnect calls usb_host_close
usb_host_close iterates over endpoints, but usb_linux_update_endp_table
  has not been called to initialize the endpoints at this points
usb_host_close calls usb_device_detach, but not attached yet
usb_host_close does an not needed ioctl(dev-fd, USBDEVFS_RESET);
usb_host_closes the fd
 usb_host_open jumps to fail, closes the fd *again*

All of this is does not lead to any real user visible bugs,
but from a code flow pov it is wrong.

Regards,

Hans

[Qemu-devel] [PATCH 05/14] usb-linux: Don't call usb_host_close when usb_host_open fails

[Hans de Goede]

---
 usb-linux.c |7 +--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index e6c6138..544aea3 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -363,13 +363,16 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, 
int configuration)
 const char *op = NULL;
 int dev_descr_len, config_descr_len;
 int interface, nb_interfaces;
-int ret, i;
+int ret, i, close_on_nodev;
 
 if (configuration == 0) /* address state - ignore */
 return 1;
 
 DPRINTF(husb: claiming interfaces. config %d\n, configuration);
 
+/* Don't close on nodev when called from usb_host_open (which passes -1) */
+close_on_nodev = (configuration == -1) ? 0 : 1;
+
 i = 0;
 dev_descr_len = dev-descr[0];
 if (dev_descr_len  dev-descr_len) {
@@ -445,7 +448,7 @@ static int usb_host_claim_interfaces(USBHostDevice *dev, 
int configuration)
 return 1;
 
 fail:
-if (errno == ENODEV) {
+if (errno == ENODEV  close_on_nodev) {
 do_disconnect(dev);
 }
 perror(op);
-- 
1.7.5.1