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
[-- Attachment #1 --]
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
[-- Attachment #2 --]
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);
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201204172205.43267.hselasky>