[libvirt] [PATCH] usbFindBusByVendor: don't leak a DIR buffer and FD

[Jim Meyering]

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 at redhat.com>
>> 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 at redhat.com>
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