util: use virDirRead API

In making the conversion to the new API, I fixed a couple bugs:
virSCSIDeviceGetSgName would leak memory if a directory
unexpectedly contained multiple entries;
virNetDevTapGetRealDeviceName could report a spurious error
from a stale errno inherited before starting the readdir search.

The decision on whether to store the result of virDirRead into
a variable is based on whether the end of the loop falls through
to cleanup code automatically.  In some cases, we have loops that
are documented to return NULL on failure, and which raise an
error on most failure paths but not in the case where the directory
was unexpectedly empty; it may be worth a followup patch to
explicitly report an error if readdir was successful but the
directory was empty, so that a NULL return always has an error set.

* src/util/vircgroup.c (virCgroupRemoveRecursively): Use new
interface.
(virCgroupKillRecursiveInternal, virCgroupSetOwner): Report
readdir failures.
* src/util/virfile.c (virFileLoopDeviceOpenSearch)
(virFileNBDDeviceFindUnused, virFileDeleteTree): Use new
interface.
* src/util/virnetdevtap.c (virNetDevTapGetRealDeviceName):
Properly check readdir errors.
* src/util/virpci.c (virPCIDeviceIterDevices)
(virPCIDeviceFileIterate, virPCIGetNetName): Report readdir
failures.
(virPCIDeviceAddressIOMMUGroupIterate): Use new interface.
* src/util/virscsi.c (virSCSIDeviceGetSgName): Report readdir
failures, and avoid memory leak.
(virSCSIDeviceGetDevName): Report readdir failures.
* src/util/virusb.c (virUSBDeviceSearch): Report readdir
failures.
* src/util/virutil.c (virGetFCHostNameByWWN)
(virFindFCHostCapableVport): Report readdir failures.

Signed-off-by: Eric Blake <eblake@redhat.com>

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index c7ebf8d2706f8e87d2f2ba5fefd0e70023e2f3d7..fce380ab5f6f7ea87462a6ce5d818d4a3625555b 100644 (file)
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -3137,6 +3137,7 @@ virCgroupRemoveRecursively(char *grppath)
     DIR *grpdir;
     struct dirent *ent;
     int rc = 0;
+    int direrr;
 
     grpdir = opendir(grppath);
     if (grpdir == NULL) {
@@ -3147,17 +3148,11 @@ virCgroupRemoveRecursively(char *grppath)
         return rc;
     }
 
-    for (;;) {
+    /* This is best-effort cleanup: we want to log failures with just
+     * VIR_ERROR instead of normal virReportError */
+    while ((direrr = virDirRead(grpdir, &ent, NULL)) > 0) {
         char *path;
 
-        errno = 0;
-        ent = readdir(grpdir);
-        if (ent == NULL) {
-            if ((rc = -errno))
-                VIR_ERROR(_("Failed to readdir for %s (%d)"), grppath, errno);
-            break;
-        }
-
         if (ent->d_name[0] == '.') continue;
         if (ent->d_type != DT_DIR) continue;
 
@@ -3170,6 +3165,11 @@ virCgroupRemoveRecursively(char *grppath)
         if (rc != 0)
             break;
     }
+    if (direrr < 0) {
+        rc = -errno;
+        VIR_ERROR(_("Failed to readdir for %s (%d)"), grppath, errno);
+    }
+
     closedir(grpdir);
 
     VIR_DEBUG("Removing cgroup %s", grppath);
@@ -3373,6 +3373,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
     DIR *dp;
     virCgroupPtr subgroup = NULL;
     struct dirent *ent;
+    int direrr;
     VIR_DEBUG("group=%p path=%s signum=%d pids=%p",
               group, group->path, signum, pids);
 
@@ -3396,7 +3397,7 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
         return -1;
     }
 
-    while ((ent = readdir(dp))) {
+    while ((direrr = virDirRead(dp, &ent, keypath)) > 0) {
         if (STREQ(ent->d_name, "."))
             continue;
         if (STREQ(ent->d_name, ".."))
@@ -3420,6 +3421,8 @@ virCgroupKillRecursiveInternal(virCgroupPtr group,
 
         virCgroupFree(&subgroup);
     }
+    if (direrr < 0)
+        goto cleanup;
 
  done:
     ret = killedAny ? 1 : 0;
@@ -3701,6 +3704,7 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
     size_t i;
     char *base = NULL, *entry = NULL;
     DIR *dh = NULL;
+    int direrr;
 
     for (i = 0; i < VIR_CGROUP_CONTROLLER_LAST; i++) {
         struct dirent *de;
@@ -3721,7 +3725,7 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
             goto cleanup;
         }
 
-        while ((de = readdir(dh)) != NULL) {
+        while ((direrr = virDirRead(dh, &de, base)) > 0) {
             if (STREQ(de->d_name, ".") ||
                 STREQ(de->d_name, ".."))
                 continue;
@@ -3738,6 +3742,8 @@ int virCgroupSetOwner(virCgroupPtr cgroup,
 
             VIR_FREE(entry);
         }
+        if (direrr < 0)
+            goto cleanup;
 
         if (chown(base, uid, gid) < 0) {
             virReportSystemError(errno,

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 34811243c892b58bacd719dbfc491c9e08b88604..bb1e7ba7bb602b65bbd3982e0abb5f34043d52e7 100644 (file)
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -607,6 +607,7 @@ static int virFileLoopDeviceOpenSearch(char **dev_name)
     struct dirent *de;
     char *looppath = NULL;
     struct loop_info64 lo;
+    int direrr;
 
     VIR_DEBUG("Looking for loop devices in /dev");
 
@@ -616,8 +617,7 @@ static int virFileLoopDeviceOpenSearch(char **dev_name)
         goto cleanup;
     }
 
-    errno = 0;
-    while ((de = readdir(dh)) != NULL) {
+    while ((direrr = virDirRead(dh, &de, "/dev")) > 0) {
         /* Checking 'loop' prefix is insufficient, since
          * new kernels have a dev named 'loop-control'
          */
@@ -650,15 +650,11 @@ static int virFileLoopDeviceOpenSearch(char **dev_name)
         /* Oh well, try the next device */
         VIR_FORCE_CLOSE(fd);
         VIR_FREE(looppath);
-        errno = 0;
     }
-
-    if (errno != 0)
-        virReportSystemError(errno, "%s",
-                             _("Unable to iterate over loop devices"));
-    else
-        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
-                       _("Unable to find a free loop device in /dev"));
+    if (direrr < 0)
+        goto cleanup;
+    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                   _("Unable to find a free loop device in /dev"));
 
  cleanup:
     if (fd != -1) {
@@ -781,6 +777,7 @@ virFileNBDDeviceFindUnused(void)
     DIR *dh;
     char *ret = NULL;
     struct dirent *de;
+    int direrr;
 
     if (!(dh = opendir(SYSFS_BLOCK_DIR))) {
         virReportSystemError(errno,
@@ -789,8 +786,7 @@ virFileNBDDeviceFindUnused(void)
         return NULL;
     }
 
-    errno = 0;
-    while ((de = readdir(dh)) != NULL) {
+    while ((direrr = virDirRead(dh, &de, SYSFS_BLOCK_DIR)) > 0) {
         if (STRPREFIX(de->d_name, "nbd")) {
             int rv = virFileNBDDeviceIsBusy(de->d_name);
             if (rv < 0)
@@ -801,15 +797,11 @@ virFileNBDDeviceFindUnused(void)
                 goto cleanup;
             }
         }
-        errno = 0;
     }
-
-    if (errno != 0)
-        virReportSystemError(errno, "%s",
-                             _("Unable to iterate over NBD devices"));
-    else
-        virReportSystemError(EBUSY, "%s",
-                             _("No free NBD devices"));
+    if (direrr < 0)
+        goto cleanup;
+    virReportSystemError(EBUSY, "%s",
+                         _("No free NBD devices"));
 
  cleanup:
     closedir(dh);
@@ -918,6 +910,7 @@ int virFileDeleteTree(const char *dir)
     struct dirent *de;
     char *filepath = NULL;
     int ret = -1;
+    int direrr;
 
     if (!dh) {
         virReportSystemError(errno, _("Cannot open dir '%s'"),
@@ -925,8 +918,7 @@ int virFileDeleteTree(const char *dir)
         return -1;
     }
 
-    errno = 0;
-    while ((de = readdir(dh)) != NULL) {
+    while ((direrr = virDirRead(dh, &de, dir)) > 0) {
         struct stat sb;
 
         if (STREQ(de->d_name, ".") ||
@@ -956,14 +948,9 @@ int virFileDeleteTree(const char *dir)
         }
 
         VIR_FREE(filepath);
-        errno = 0;
     }
-
-    if (errno) {
-        virReportSystemError(errno, _("Cannot read dir '%s'"),
-                             dir);
+    if (direrr < 0)
         goto cleanup;
-    }
 
     if (rmdir(dir) < 0 && errno != ENOENT) {
         virReportSystemError(errno,

diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 03bb4185694e4eda6ed0a3dbf71489f1d66362e6..0b444fad39627481b96b83cb539fa3a5f7e80f5f 100644 (file)
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2007-2013 Red Hat, Inc.
+ * Copyright (C) 2007-2014 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
@@ -105,7 +105,7 @@ virNetDevTapGetRealDeviceName(char *ifname ATTRIBUTE_UNUSED)
         return NULL;
     }
 
-    while ((dp = readdir(dirp)) != NULL) {
+    while (virDirRead(dirp, &dp, "/dev") > 0) {
         if (STRPREFIX(dp->d_name, "tap")) {
             struct ifreq ifr;
             if (virAsprintf(&devpath, "/dev/%s", dp->d_name) < 0) {
@@ -139,14 +139,8 @@ virNetDevTapGetRealDeviceName(char *ifname ATTRIBUTE_UNUSED)
             VIR_FREE(devpath);
             VIR_FORCE_CLOSE(fd);
         }
-
-        errno = 0;
     }
 
-    if (errno != 0)
-        virReportSystemError(errno, "%s",
-                             _("Unable to iterate over TAP devices"));
-
  cleanup:
     VIR_FREE(devpath);
     VIR_FORCE_CLOSE(fd);

diff --git a/src/util/virpci.c b/src/util/virpci.c
index af0bfa8b38f424125e84f42f3210f31b2846b92b..e0f2344286bfdcc07cf04b6b66852c4a6ce351c9 100644 (file)
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1,7 +1,7 @@
 /*
  * virpci.c: helper APIs for managing host PCI devices
  *
- * Copyright (C) 2009-2013 Red Hat, Inc.
+ * Copyright (C) 2009-2014 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
@@ -445,7 +445,7 @@ virPCIDeviceIterDevices(virPCIDeviceIterPredicate predicate,
         return -1;
     }
 
-    while ((entry = readdir(dir))) {
+    while ((ret = virDirRead(dir, &entry, PCI_SYSFS "devices")) > 0) {
         unsigned int domain, bus, slot, function;
         virPCIDevicePtr check;
         char *tmp;
@@ -1907,6 +1907,7 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev,
     DIR *dir = NULL;
     int ret = -1;
     struct dirent *ent;
+    int direrr;
 
     if (virAsprintf(&pcidir, "/sys/bus/pci/devices/%04x:%02x:%02x.%x",
                     dev->domain, dev->bus, dev->slot, dev->function) < 0)
@@ -1918,7 +1919,7 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev,
         goto cleanup;
     }
 
-    while ((ent = readdir(dir)) != NULL) {
+    while ((direrr = virDirRead(dir, &ent, pcidir)) > 0) {
         /* Device assignment requires:
          *   $PCIDIR/config, $PCIDIR/resource, $PCIDIR/resourceNNN,
          *   $PCIDIR/rom, $PCIDIR/reset
@@ -1935,6 +1936,8 @@ int virPCIDeviceFileIterate(virPCIDevicePtr dev,
             VIR_FREE(file);
         }
     }
+    if (direrr < 0)
+        goto cleanup;
 
     ret = 0;
 
@@ -1961,6 +1964,7 @@ virPCIDeviceAddressIOMMUGroupIterate(virPCIDeviceAddressPtr orig,
     DIR *groupDir = NULL;
     int ret = -1;
     struct dirent *ent;
+    int direrr;
 
     if (virAsprintf(&groupPath,
                     PCI_SYSFS "devices/%04x:%02x:%02x.%x/iommu_group/devices",
@@ -1973,7 +1977,7 @@ virPCIDeviceAddressIOMMUGroupIterate(virPCIDeviceAddressPtr orig,
         goto cleanup;
     }
 
-    while ((errno = 0, ent = readdir(groupDir)) != NULL) {
+    while ((direrr = virDirRead(groupDir, &ent, groupPath)) > 0) {
         virPCIDeviceAddress newDev;
 
         if (ent->d_name[0] == '.')
@@ -1989,12 +1993,8 @@ virPCIDeviceAddressIOMMUGroupIterate(virPCIDeviceAddressPtr orig,
         if ((actor)(&newDev, opaque) < 0)
             goto cleanup;
     }
-    if (errno != 0) {
-        virReportSystemError(errno,
-                             _("Failed to read directory entry for %s"),
-                             groupPath);
+    if (direrr < 0)
         goto cleanup;
-    }
 
     ret = 0;
 
@@ -2638,7 +2638,7 @@ virPCIGetNetName(char *device_link_sysfs_path, char **netname)
     if (dir == NULL)
         goto out;
 
-    while ((entry = readdir(dir))) {
+    while (virDirRead(dir, &entry, pcidev_sysfs_net_path) > 0) {
         if (STREQ(entry->d_name, ".") ||
             STREQ(entry->d_name, ".."))
             continue;

diff --git a/src/util/virscsi.c b/src/util/virscsi.c
index 572b818e05a4dbcdd6e180e70f7d6d43c4e8de4f..9a0205ff886a55836de249c5a29180eea48d0a79 100644 (file)
--- a/src/util/virscsi.c
+++ b/src/util/virscsi.c
@@ -133,12 +133,13 @@ virSCSIDeviceGetSgName(const char *sysfs_prefix,
         goto cleanup;
     }
 
-    while ((entry = readdir(dir))) {
+    while (virDirRead(dir, &entry, path) > 0) {
         if (entry->d_name[0] == '.')
             continue;
 
-        if (VIR_STRDUP(sg, entry->d_name) < 0)
-            goto cleanup;
+        /* Assume a single directory entry */
+        ignore_value(VIR_STRDUP(sg, entry->d_name));
+        break;
     }
 
  cleanup:
@@ -178,7 +179,7 @@ virSCSIDeviceGetDevName(const char *sysfs_prefix,
         goto cleanup;
     }
 
-    while ((entry = readdir(dir))) {
+    while (virDirRead(dir, &entry, path) > 0) {
         if (entry->d_name[0] == '.')
             continue;
 

diff --git a/src/util/virusb.c b/src/util/virusb.c
index d959f5fd5c7742b3381f444a5dad2f2cc27dba98..8244771a9f5c1f85745af9b59723bfc42bb858a0 100644 (file)
--- a/src/util/virusb.c
+++ b/src/util/virusb.c
@@ -1,7 +1,7 @@
 /*
  * virusb.c: helper APIs for managing host USB devices
  *
- * Copyright (C) 2009-2013 Red Hat, Inc.
+ * Copyright (C) 2009-2014 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
@@ -134,6 +134,7 @@ virUSBDeviceSearch(unsigned int vendor,
     struct dirent *de;
     virUSBDeviceListPtr list = NULL, ret = NULL;
     virUSBDevicePtr usb;
+    int direrr;
 
     if (!(list = virUSBDeviceListNew()))
         goto cleanup;
@@ -146,7 +147,7 @@ virUSBDeviceSearch(unsigned int vendor,
         goto cleanup;
     }
 
-    while ((de = readdir(dir))) {
+    while ((direrr = virDirRead(dir, &de, USB_SYSFS "/devices")) > 0) {
         unsigned int found_prod, found_vend, found_bus, found_devno;
         char *tmpstr = de->d_name;
 
@@ -197,6 +198,8 @@ virUSBDeviceSearch(unsigned int vendor,
         if (found)
             break;
     }
+    if (direrr < 0)
+        goto cleanup;
     ret = list;
 
  cleanup:

diff --git a/src/util/virutil.c b/src/util/virutil.c
index 9be15907b881aecd598a9c0fe3c75c124c1cec48..875e2b0f1a7ecb19878f1a320e1e17948cba292b 100644 (file)
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1867,7 +1867,7 @@ virGetFCHostNameByWWN(const char *sysfs_prefix,
             p = buf;                                  \
     } while (0)
 
-    while ((entry = readdir(dir))) {
+    while (virDirRead(dir, &entry, prefix) > 0) {
         if (entry->d_name[0] == '.')
             continue;
 
@@ -1949,7 +1949,7 @@ virFindFCHostCapableVport(const char *sysfs_prefix)
         return NULL;
     }
 
-    while ((entry = readdir(dir))) {
+    while (virDirRead(dir, &entry, prefix) > 0) {
         unsigned int host;
         char *p = NULL;