Date: Tue, 17 Apr 2012 21:58:36 +0200 From: Hans Petter Selasky <hselasky@c2i.net> To: Matthias Apitz <guru@unixarea.de> Cc: Bruce Cran <bruce@cran.org.uk>, freebsd-usb@freebsd.org Subject: Re: dfu-util 0.5 Message-ID: <201204172158.36499.hselasky@c2i.net> In-Reply-To: <20120417185216.GA1306@tiny> References: <20120417100147.GA2557@tiny> <201204172041.56552.hselasky@c2i.net> <20120417185216.GA1306@tiny>
next in thread | previous in thread | raw e-mail | index | archive | help
--Boundary-00=_stcjPEB6qXT1vpE
Content-Type: Text/Plain;
charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
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
--Boundary-00=_stcjPEB6qXT1vpE
Content-Type: text/x-patch;
charset="iso-8859-1";
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..11ddf4f 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)
{
@@ -83,24 +83,25 @@ 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;
+ dfu_if->dev_handle = devh;
dfu_if->vendor =
desc.idVendor;
dfu_if->product =
@@ -115,17 +116,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 +150,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 +170,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 +192,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 +253,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 +289,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 +947,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=_stcjPEB6qXT1vpE--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201204172158.36499.hselasky>