Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 17 Apr 2012 22:05:43 +0200
From:      Hans Petter Selasky <hselasky@c2i.net>
To:        freebsd-usb@freebsd.org
Cc:        Bruce Cran <bruce@cran.org.uk>
Subject:   Re: dfu-util 0.5
Message-ID:  <201204172205.43267.hselasky@c2i.net>
In-Reply-To: <201204172158.36499.hselasky@c2i.net>
References:  <20120417100147.GA2557@tiny> <20120417185216.GA1306@tiny> <201204172158.36499.hselasky@c2i.net>

next in thread | previous in thread | raw e-mail | index | archive | help
--Boundary-00=_X0cjPFIooSR/hBr
Content-Type: Text/Plain;
  charset="iso-8859-15"
Content-Transfer-Encoding: 7bit

On Tuesday 17 April 2012 21:58:36 Hans Petter Selasky wrote:
> Hi Matthias,
> 
> I reviewed the dfu-util code and there is a bug there with regard to libusb
> usage.
> 
> Can you try the attached patch. It fixes a duplicate libusb open bug.
> 
> --HPS

Found a small bug. Try updated patch.

--HPS

--Boundary-00=_X0cjPFIooSR/hBr
Content-Type: text/x-patch;
  charset="iso-8859-15";
  name="dfu-util.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
	filename="dfu-util.patch"

diff --git a/src/main.c b/src/main.c
index 8986859..46f0e01 100644
--- a/src/main.c
+++ b/src/main.c
@@ -63,7 +63,7 @@ int verbose = 0;
  * Iterate through all DFU interfaces and their alternate settings
  * and call the passed handler function on each setting until handler
  * returns non-zero. */
-static int find_dfu_if(libusb_device *dev,
+static int find_dfu_if(libusb_device *dev, libusb_device_handle *devh,
 		       int (*handler)(struct dfu_if *, void *),
 		       void *v)
 {
@@ -76,6 +76,9 @@ static int find_dfu_if(libusb_device *dev,
 	int rc;
 
 	memset(dfu_if, 0, sizeof(*dfu_if));
+
+	dfu_if->dev_handle = devh;
+
 	rc = libusb_get_device_descriptor(dev, &desc);
 	if (rc)
 		return rc;
@@ -83,21 +86,21 @@ static int find_dfu_if(libusb_device *dev,
 	     cfg_idx++) {
 		rc = libusb_get_config_descriptor(dev, cfg_idx, &cfg);
 		if (rc)
-			return rc;
+			goto done;	
 		/* in some cases, noticably FreeBSD if uid != 0,
 		 * the configuration descriptors are empty */
 		if (!cfg)
-			return 0;
+			goto done;
 		for (intf_idx = 0; intf_idx < cfg->bNumInterfaces;
 		     intf_idx++) {
 			uif = &cfg->interface[intf_idx];
 			if (!uif)
-				return 0;
+				goto done;
 			for (alt_idx = 0;
 			     alt_idx < uif->num_altsetting; alt_idx++) {
 				intf = &uif->altsetting[alt_idx];
 				if (!intf)
-					return 0;
+					goto done;
 				if (intf->bInterfaceClass == 0xfe &&
 				    intf->bInterfaceSubClass == 1) {
 					dfu_if->dev = dev;
@@ -115,17 +118,21 @@ static int find_dfu_if(libusb_device *dev,
 						dfu_if->flags |= DFU_IFF_DFU;
 					else
 						dfu_if->flags &= ~DFU_IFF_DFU;
-					if (!handler)
-						return 1;
+					if (!handler) {
+						rc = 1;
+						goto done;
+					}
 					rc = handler(dfu_if, v);
 					if (rc != 0)
-						return rc;
+						goto done;
 				}
 			}
 		}
-
 		libusb_free_config_descriptor(cfg);
 	}
+done:
+	if (devh == NULL && dfu_if->dev_handle != NULL)
+		libusb_close(dfu_if->dev_handle);
 
 	return 0;
 }
@@ -145,7 +152,8 @@ static int _get_first_cb(struct dfu_if *dif, void *v)
 /* Fills in dif with the first found DFU interface */
 static int get_first_dfu_if(struct dfu_if *dif)
 {
-	return find_dfu_if(dif->dev, &_get_first_cb, (void *) dif);
+	return find_dfu_if(dif->dev, dif->dev_handle,
+	    &_get_first_cb, (void *) dif);
 }
 
 static int _check_match_cb(struct dfu_if *dif, void *v)
@@ -164,7 +172,8 @@ static int _check_match_cb(struct dfu_if *dif, void *v)
 /* Fills in dif from the matching DFU interface/altsetting */
 static int get_matching_dfu_if(struct dfu_if *dif)
 {
-	return find_dfu_if(dif->dev, &_check_match_cb, (void *) dif);
+	return find_dfu_if(dif->dev, dif->dev_handle,
+	    &_check_match_cb, (void *) dif);
 }
 
 static int _count_match_cb(struct dfu_if *dif, void *v)
@@ -185,7 +194,8 @@ static int _count_match_cb(struct dfu_if *dif, void *v)
 static int count_matching_dfu_if(struct dfu_if *dif)
 {
 	dif->count = 0;
-	find_dfu_if(dif->dev, &_count_match_cb, (void *) dif);
+	find_dfu_if(dif->dev, dif->dev_handle,
+	    &_count_match_cb, (void *) dif);
 	return dif->count;
 }
 
@@ -245,7 +255,7 @@ static int list_dfu_interfaces(libusb_context *ctx)
 
 	for (i = 0; i < num_devs; ++i) {
 		dev = list[i];
-		find_dfu_if(dev, &print_dfu_if, NULL);
+		find_dfu_if(dev, NULL, &print_dfu_if, NULL);
 	}
 
 	libusb_free_device_list(list, 1);
@@ -281,7 +291,7 @@ static int count_dfu_interfaces(libusb_device *dev)
 {
 	int num_found = 0;
 
-	find_dfu_if(dev, &_count_cb, (void *) &num_found);
+	find_dfu_if(dev, NULL, &_count_cb, (void *) &num_found);
 
 	return num_found;
 }
@@ -939,7 +949,8 @@ dfustate:
 	if (alt_name) {
 		int n;
 
-		n = find_dfu_if(dif->dev, &alt_by_name, alt_name);
+		n = find_dfu_if(dif->dev, dif->dev_handle,
+		    &alt_by_name, alt_name);
 		if (!n) {
 			fprintf(stderr, "No such Alternate Setting: \"%s\"\n",
 			    alt_name);

--Boundary-00=_X0cjPFIooSR/hBr--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201204172205.43267.hselasky>