Re: [PATCH] util: Implement and use virFileIsRegular() rather than d_type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/06/2018 11:37 AM, Stefan Berger wrote:
The dirent's d_type field is not portable to all platforms. So we have
to use stat() to determine the type of file for the functions that need
to be cross-platform. Fix virFileChownFiles() by calling the new
virFileIsRegular() function.

Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>
---
  src/libvirt_private.syms |  1 +
  src/util/virfile.c       | 17 +++++++++++++----
  src/util/virfile.h       |  1 +
  3 files changed, 15 insertions(+), 4 deletions(-)

+
+bool
+virFileIsRegular(const char *path)
+{
+    struct stat s;
+    return (stat(path, &s) == 0) && S_ISREG(s.st_mode);

This always does a stat.

+}
+
+
  /**
   * virFileExists: Check for presence of file
   * @path: Path of file to check
@@ -3005,12 +3014,13 @@ int virFileChownFiles(const char *name,
          return -1;
while ((direrr = virDirRead(dir, &ent, name)) > 0) {
-        if (ent->d_type != DT_REG)
-            continue;

And this was the non-portable code you're getting rid of, which was already buggy - even on systems with d_type, a return of DT_UNKNOWN requires a stat() rather than a blind continue. However, your replacement is pessimizing platforms where d_type is reliable, by making us do a stat() where we previously did not need to.

Better would be using macros that use d_type where possible, and fall back to stat() only when d_type does not exist or is DT_UNKNOWN; for example, here's how gnulib does it:

#if HAVE_STRUCT_DIRENT_D_TYPE
/* True if the type of the directory entry D is known.  */
# define DT_IS_KNOWN(d) ((d)->d_type != DT_UNKNOWN)
/* True if the type of the directory entry D must be T.  */
# define DT_MUST_BE(d, t) ((d)->d_type == (t))
# define D_TYPE(d) ((d)->d_type)
#else
# define DT_IS_KNOWN(d) false
# define DT_MUST_BE(d, t) false
# define D_TYPE(d) DT_UNKNOWN

# undef DT_UNKNOWN
# define DT_UNKNOWN 0

(although we have to be careful with licensing, because that is from lib/fts.c which is marked GPLv3+).

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux