Daniel P. Berrange wrote:
> On Mon, Jan 25, 2010 at 04:37:43PM +0100, Jim Meyering wrote:
>> I know better than to say this is so simple/obvious that
>> it doesn't need review, but I'm tempted...
>>
>> >From 02c7dfa830544bbafd62867fc70b3aba7cc07385 Mon Sep 17 00:00:00 2001
>> From: Jim Meyering <meyering@xxxxxxxxxx>
>> Date: Mon, 25 Jan 2010 16:36:07 +0100
>> Subject: [PATCH] usbFindBusByVendor: don't leak a DIR buffer and FD
>>
>> * src/util/hostusb.c (usbFindBusByVendor): Don't leak a DIR buffer
>> and file descriptor.
>> ---
>> src/util/hostusb.c | 3 ++-
>> 1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/util/hostusb.c b/src/util/hostusb.c
>> index 8fbb486..9a37103 100644
>> --- a/src/util/hostusb.c
>> +++ b/src/util/hostusb.c
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (C) 2009 Red Hat, Inc.
>> + * Copyright (C) 2009-2010 Red Hat, Inc.
>> *
>> * This library is free software; you can redistribute it and/or
>> * modify it under the terms of the GNU Lesser General Public
>> @@ -151,6 +151,7 @@ static int usbFindBusByVendor(virConnectPtr conn,
>> ret = 0;
>>
>> error:
>> + closedir (dir);
>> return ret;
>> }
>
> ACK, that context is a little misleading because the original code 'error:'
> should have been 'cleanup:' really, since it is a shared success/failure
> path, not a separate error path.
I've pushed the above.
While doing that, I realized that closedir(NULL) might not go down well.
This fixes that and renames the labels in two functions.
>From 351cb1fadf1e02d4b1a6f00bfadd9d992b2eb75c Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@xxxxxxxxxx>
Date: Mon, 25 Jan 2010 16:54:06 +0100
Subject: [PATCH] hostusb: closedir only if non-NULL; rename labels: s/error/cleanup/
* src/util/hostusb.c (usbSysReadFile): Rename labels s/error/cleanup/
(usbFindBusByVendor): Likewise. And closedir only if non-NULL.
---
src/util/hostusb.c | 23 ++++++++++++-----------
1 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/src/util/hostusb.c b/src/util/hostusb.c
index 9a37103..a452abd 100644
--- a/src/util/hostusb.c
+++ b/src/util/hostusb.c
@@ -70,20 +70,20 @@ static int usbSysReadFile(virConnectPtr conn,
tmp = virAsprintf(&filename, USB_SYSFS "/devices/%s/%s", d_name, f_name);
if (tmp < 0) {
virReportOOMError(conn);
- goto error;
+ goto cleanup;
}
if (virFileReadAll(filename, 1024, &buf) < 0)
- goto error;
+ goto cleanup;
if (virStrToLong_ui(buf, &ignore, base, value) < 0) {
usbReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("Could not parse usb file %s"), filename);
- goto error;
+ goto cleanup;
}
ret = 0;
-error:
+cleanup:
VIR_FREE(filename);
VIR_FREE(buf);
return ret;
@@ -103,7 +103,7 @@ static int usbFindBusByVendor(virConnectPtr conn,
virReportSystemError(conn, errno,
_("Could not open directory %s"),
USB_SYSFS "/devices");
- goto error;
+ goto cleanup;
}
while ((de = readdir(dir))) {
@@ -113,10 +113,10 @@ static int usbFindBusByVendor(virConnectPtr conn,
if (usbSysReadFile(conn, "idVendor", de->d_name,
16, &found_vend) < 0)
- goto error;
+ goto cleanup;
if (usbSysReadFile(conn, "idProduct", de->d_name,
16, &found_prod) < 0)
- goto error;
+ goto cleanup;
if (found_prod == product && found_vend == vendor) {
/* Lookup bus.addr info */
@@ -130,12 +130,12 @@ static int usbFindBusByVendor(virConnectPtr conn,
usbReportError(conn, VIR_ERR_INTERNAL_ERROR,
_("Failed to parse dir name '%s'"),
de->d_name);
- goto error;
+ goto cleanup;
}
if (usbSysReadFile(conn, "devnum", de->d_name,
10, &found_addr) < 0)
- goto error;
+ goto cleanup;
*bus = found_bus;
*devno = found_addr;
@@ -150,8 +150,9 @@ static int usbFindBusByVendor(virConnectPtr conn,
else
ret = 0;
-error:
- closedir (dir);
+cleanup:
+ if (dir)
+ closedir (dir);
return ret;
}
--
1.7.0.rc0.127.gab8271
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list