Introduce Variants of Some Functions that Take Optional File Descriptors

Somewhat surprisingly, calling `fchmod()` is non-trivially faster than calling
`chmod()`, and so on.

This commit introduces alternate variants to some common file util functions
that take an optional fd. If present, they call the `f`-variant of the
underlying function. Otherwise, they fall back to the regular filename-taking
version of the function.

diff --git a/programs/util.c b/programs/util.c
index e017772ef6e6b49c13282c47b197aa5216b3af7c..a3f5564afa0de72ea4fdacfe6a18d6894c58898a 100644 (file)
--- a/programs/util.c
+++ b/programs/util.c
@@ -102,6 +102,17 @@ UTIL_STATIC void* UTIL_realloc(void *ptr, size_t size)
     #define chmod _chmod
 #endif
 
+#ifndef ZSTD_HAVE_FCHMOD
+#if PLATFORM_POSIX_VERSION >= 199309L
+#define ZSTD_HAVE_FCHMOD
+#endif
+#endif
+
+#ifndef ZSTD_HAVE_FCHOWN
+#if PLATFORM_POSIX_VERSION >= 200809L
+#define ZSTD_HAVE_FCHOWN
+#endif
+#endif
 
 /*-****************************************
 *  Console log
@@ -147,21 +158,38 @@ void UTIL_traceFileStat(void)
     g_traceFileStat = 1;
 }
 
-int UTIL_stat(const char* filename, stat_t* statbuf)
+int UTIL_fstat(const int fd, const char* filename, stat_t* statbuf)
 {
     int ret;
-    UTIL_TRACE_CALL("UTIL_stat(%s)", filename);
+    UTIL_TRACE_CALL("UTIL_stat(%d, %s)", fd, filename);
 #if defined(_MSC_VER)
-    ret = !_stat64(filename, statbuf);
+    if (fd >= 0) {
+        ret = !_fstat64(fd, statbuf);
+    } else {
+        ret = !_stat64(filename, statbuf);
+    }
 #elif defined(__MINGW32__) && defined (__MSVCRT__)
-    ret = !_stati64(filename, statbuf);
+    if (fd >= 0) {
+        ret = !_fstati64(fd, statbuf);
+    } else {
+        ret = !_stati64(filename, statbuf);
+    }
 #else
-    ret = !stat(filename, statbuf);
+    if (fd >= 0) {
+        ret = !fstat(fd, statbuf);
+    } else {
+        ret = !stat(filename, statbuf);
+    }
 #endif
     UTIL_TRACE_RET(ret);
     return ret;
 }
 
+int UTIL_stat(const char* filename, stat_t* statbuf)
+{
+    return UTIL_fstat(-1, filename, statbuf);
+}
+
 int UTIL_isRegularFile(const char* infilename)
 {
     stat_t statbuf;
@@ -183,11 +211,16 @@ int UTIL_isRegularFileStat(const stat_t* statbuf)
 
 /* like chmod, but avoid changing permission of /dev/null */
 int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions)
+{
+    return UTIL_fchmod(-1, filename, statbuf, permissions);
+}
+
+int UTIL_fchmod(const int fd, char const* filename, const stat_t* statbuf, mode_t permissions)
 {
     stat_t localStatBuf;
     UTIL_TRACE_CALL("UTIL_chmod(%s, %#4o)", filename, (unsigned)permissions);
     if (statbuf == NULL) {
-        if (!UTIL_stat(filename, &localStatBuf)) {
+        if (!UTIL_fstat(fd, filename, &localStatBuf)) {
             UTIL_TRACE_RET(0);
             return 0;
         }
@@ -197,9 +230,20 @@ int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions)
         UTIL_TRACE_RET(0);
         return 0; /* pretend success, but don't change anything */
     }
-    UTIL_TRACE_CALL("chmod");
+#ifdef ZSTD_HAVE_FCHMOD
+    if (fd >= 0) {
+        int ret;
+        UTIL_TRACE_CALL("fchmod");
+        ret = fchmod(fd, permissions);
+        UTIL_TRACE_RET(ret);
+        UTIL_TRACE_RET(ret);
+        return ret;
+    } else
+#endif
     {
-        int const ret = chmod(filename, permissions);
+        int ret;
+        UTIL_TRACE_CALL("chmod");
+        ret = chmod(filename, permissions);
         UTIL_TRACE_RET(ret);
         UTIL_TRACE_RET(ret);
         return ret;
@@ -236,12 +280,17 @@ int UTIL_utime(const char* filename, const stat_t *statbuf)
 }
 
 int UTIL_setFileStat(const char *filename, const stat_t *statbuf)
+{
+    return UTIL_setFDStat(-1, filename, statbuf);
+}
+
+int UTIL_setFDStat(const int fd, const char *filename, const stat_t *statbuf)
 {
     int res = 0;
     stat_t curStatBuf;
-    UTIL_TRACE_CALL("UTIL_setFileStat(%s)", filename);
+    UTIL_TRACE_CALL("UTIL_setFileStat(%d, %s)", fd, filename);
 
-    if (!UTIL_stat(filename, &curStatBuf) || !UTIL_isRegularFileStat(&curStatBuf)) {
+    if (!UTIL_fstat(fd, filename, &curStatBuf) || !UTIL_isRegularFileStat(&curStatBuf)) {
         UTIL_TRACE_RET(-1);
         return -1;
     }
@@ -258,13 +307,27 @@ int UTIL_setFileStat(const char *filename, const stat_t *statbuf)
      * setgid bits." */
 
 #if !defined(_WIN32)
-    res += chown(filename, -1, statbuf->st_gid);  /* Apply group ownership */
+#ifdef ZSTD_HAVE_FCHOWN
+    if (fd >= 0) {
+        res += fchown(fd, -1, statbuf->st_gid);  /* Apply group ownership */
+    } else
+#endif
+    {
+        res += chown(filename, -1, statbuf->st_gid);  /* Apply group ownership */
+    }
 #endif
 
-    res += UTIL_chmod(filename, &curStatBuf, statbuf->st_mode & 0777);  /* Copy file permissions */
+    res += UTIL_fchmod(fd, filename, &curStatBuf, statbuf->st_mode & 0777);  /* Copy file permissions */
 
 #if !defined(_WIN32)
-    res += chown(filename, statbuf->st_uid, -1);  /* Apply user ownership */
+#ifdef ZSTD_HAVE_FCHOWN
+    if (fd >= 0) {
+        res += fchown(fd, statbuf->st_uid, -1);  /* Apply user ownership */
+    } else
+#endif
+    {
+        res += chown(filename, statbuf->st_uid, -1);  /* Apply user ownership */
+    }
 #endif
 
     errno = 0;

diff --git a/programs/util.h b/programs/util.h
index 4ec54137dd3c3554dcdab8bf3f7dd070f72de68b..8234646bf3dc61e482f8331795b8bf54a7aa0746 100644 (file)
--- a/programs/util.h
+++ b/programs/util.h
@@ -126,15 +126,25 @@ int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg, const
 /**
  * Calls platform's equivalent of stat() on filename and writes info to statbuf.
  * Returns success (1) or failure (0).
+ *
+ * UTIL_fstat() is like UTIL_stat() but takes an optional fd that refers to the
+ * file in question. It turns out that this can be meaningfully faster. If fd is
+ * -1, behaves just like UTIL_stat() (i.e., falls back to using the filename).
  */
 int UTIL_stat(const char* filename, stat_t* statbuf);
+int UTIL_fstat(const int fd, const char* filename, stat_t* statbuf);
 
 /**
  * Instead of getting a file's stats, this updates them with the info in the
  * provided stat_t. Currently sets owner, group, atime, and mtime. Will only
  * update this info for regular files.
+ *
+ * UTIL_setFDStat() also takes an fd, and will preferentially use that to
+ * indicate which file to modify, If fd is -1, it will fall back to using the
+ * filename.
  */
 int UTIL_setFileStat(const char* filename, const stat_t* statbuf);
+int UTIL_setFDStat(const int fd, const char* filename, const stat_t* statbuf);
 
 /**
  * Set atime to now and mtime to the st_mtim in statbuf.
@@ -159,8 +169,11 @@ U64 UTIL_getFileSizeStat(const stat_t* statbuf);
  * Like chmod(), but only modifies regular files. Provided statbuf may be NULL,
  * in which case this function will stat() the file internally, in order to
  * check whether it should be modified.
+ *
+ * If fd is -1, fd is ignored and the filename is used.
  */
 int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions);
+int UTIL_fchmod(const int fd, char const* filename, const stat_t* statbuf, mode_t permissions);
 
 /*
  * In the absence of a pre-existing stat result on the file in question, these

diff --git a/tests/cli-tests/file-stat/compress-file-to-file.sh.stderr.exact b/tests/cli-tests/file-stat/compress-file-to-file.sh.stderr.exact
index a1ad09ef7058289ad9fe074bb29703ec37d0e7f6..913a99e1ed61847fd5c07adb4b29a66cb6fb8818 100644 (file)
--- a/tests/cli-tests/file-stat/compress-file-to-file.sh.stderr.exact
+++ b/tests/cli-tests/file-stat/compress-file-to-file.sh.stderr.exact
@@ -3,35 +3,35 @@ Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isConsole(2)
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_getFileSize(file)
-Trace:FileStat:  > UTIL_stat(file)
+Trace:FileStat:  > UTIL_stat(-1, file)
 Trace:FileStat:  < 1
 Trace:FileStat: < 65537
-Trace:FileStat: > UTIL_stat(file)
+Trace:FileStat: > UTIL_stat(-1, file)
 Trace:FileStat: < 1
 Trace:FileStat: > UTIL_isDirectoryStat()
 Trace:FileStat: < 0
-Trace:FileStat: > UTIL_stat(file)
+Trace:FileStat: > UTIL_stat(-1, file)
 Trace:FileStat: < 1
 Trace:FileStat: > UTIL_isSameFile(file, file.zst)
-Trace:FileStat:  > UTIL_stat(file)
+Trace:FileStat:  > UTIL_stat(-1, file)
 Trace:FileStat:  < 1
-Trace:FileStat:  > UTIL_stat(file.zst)
+Trace:FileStat:  > UTIL_stat(-1, file.zst)
 Trace:FileStat:  < 0
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isRegularFile(file.zst)
-Trace:FileStat:  > UTIL_stat(file.zst)
+Trace:FileStat:  > UTIL_stat(-1, file.zst)
 Trace:FileStat:  < 0
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isRegularFile(file.zst)
-Trace:FileStat:  > UTIL_stat(file.zst)
+Trace:FileStat:  > UTIL_stat(-1, file.zst)
 Trace:FileStat:  < 1
 Trace:FileStat: < 1
 Trace:FileStat: > UTIL_getFileSize(file)
-Trace:FileStat:  > UTIL_stat(file)
+Trace:FileStat:  > UTIL_stat(-1, file)
 Trace:FileStat:  < 1
 Trace:FileStat: < 65537
-Trace:FileStat: > UTIL_setFileStat(file.zst)
-Trace:FileStat:  > UTIL_stat(file.zst)
+Trace:FileStat: > UTIL_setFileStat(-1, file.zst)
+Trace:FileStat:  > UTIL_stat(-1, file.zst)
 Trace:FileStat:  < 1
 Trace:FileStat:  > UTIL_utime(file.zst)
 Trace:FileStat:  < 0

diff --git a/tests/cli-tests/file-stat/compress-file-to-stdout.sh.stderr.exact b/tests/cli-tests/file-stat/compress-file-to-stdout.sh.stderr.exact
index 7c690d20b8494baaf7b8b5f5aaefc887c3b77e37..a25a355723c1db74da3958bf00fba9c0ac51eb52 100644 (file)
--- a/tests/cli-tests/file-stat/compress-file-to-stdout.sh.stderr.exact
+++ b/tests/cli-tests/file-stat/compress-file-to-stdout.sh.stderr.exact
@@ -5,20 +5,20 @@ Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isConsole(2)
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_getFileSize(file)
-Trace:FileStat:  > UTIL_stat(file)
+Trace:FileStat:  > UTIL_stat(-1, file)
 Trace:FileStat:  < 1
 Trace:FileStat: < 65537
-Trace:FileStat: > UTIL_stat(file)
+Trace:FileStat: > UTIL_stat(-1, file)
 Trace:FileStat: < 1
 Trace:FileStat: > UTIL_isDirectoryStat()
 Trace:FileStat: < 0
-Trace:FileStat: > UTIL_stat(file)
+Trace:FileStat: > UTIL_stat(-1, file)
 Trace:FileStat: < 1
 Trace:FileStat: > UTIL_isRegularFile(/*stdout*\)
-Trace:FileStat:  > UTIL_stat(/*stdout*\)
+Trace:FileStat:  > UTIL_stat(-1, /*stdout*\)
 Trace:FileStat:  < 0
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_getFileSize(file)
-Trace:FileStat:  > UTIL_stat(file)
+Trace:FileStat:  > UTIL_stat(-1, file)
 Trace:FileStat:  < 1
 Trace:FileStat: < 65537

diff --git a/tests/cli-tests/file-stat/compress-stdin-to-file.sh.stderr.exact b/tests/cli-tests/file-stat/compress-stdin-to-file.sh.stderr.exact
index 00afd97c365833121848d791be203ae8a2f31694..9183c46745947d2861836bdf2c0a6f96ffe6b706 100644 (file)
--- a/tests/cli-tests/file-stat/compress-stdin-to-file.sh.stderr.exact
+++ b/tests/cli-tests/file-stat/compress-stdin-to-file.sh.stderr.exact
@@ -3,22 +3,22 @@ Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isConsole(2)
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_getFileSize(/*stdin*\)
-Trace:FileStat:  > UTIL_stat(/*stdin*\)
+Trace:FileStat:  > UTIL_stat(-1, /*stdin*\)
 Trace:FileStat:  < 0
 Trace:FileStat: < -1
 Trace:FileStat: > UTIL_isSameFile(/*stdin*\, file.zst)
-Trace:FileStat:  > UTIL_stat(/*stdin*\)
+Trace:FileStat:  > UTIL_stat(-1, /*stdin*\)
 Trace:FileStat:  < 0
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isRegularFile(file.zst)
-Trace:FileStat:  > UTIL_stat(file.zst)
+Trace:FileStat:  > UTIL_stat(-1, file.zst)
 Trace:FileStat:  < 0
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isRegularFile(file.zst)
-Trace:FileStat:  > UTIL_stat(file.zst)
+Trace:FileStat:  > UTIL_stat(-1, file.zst)
 Trace:FileStat:  < 1
 Trace:FileStat: < 1
 Trace:FileStat: > UTIL_getFileSize(/*stdin*\)
-Trace:FileStat:  > UTIL_stat(/*stdin*\)
+Trace:FileStat:  > UTIL_stat(-1, /*stdin*\)
 Trace:FileStat:  < 0
 Trace:FileStat: < -1

diff --git a/tests/cli-tests/file-stat/compress-stdin-to-stdout.sh.stderr.exact b/tests/cli-tests/file-stat/compress-stdin-to-stdout.sh.stderr.exact
index 8bf05e641e1271449e87874617c13c890dfe9ef4..66b630e433c08e5dacb69f6afd14408c5e638b99 100644 (file)
--- a/tests/cli-tests/file-stat/compress-stdin-to-stdout.sh.stderr.exact
+++ b/tests/cli-tests/file-stat/compress-stdin-to-stdout.sh.stderr.exact
@@ -5,14 +5,14 @@ Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isConsole(2)
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_getFileSize(/*stdin*\)
-Trace:FileStat:  > UTIL_stat(/*stdin*\)
+Trace:FileStat:  > UTIL_stat(-1, /*stdin*\)
 Trace:FileStat:  < 0
 Trace:FileStat: < -1
 Trace:FileStat: > UTIL_isRegularFile(/*stdout*\)
-Trace:FileStat:  > UTIL_stat(/*stdout*\)
+Trace:FileStat:  > UTIL_stat(-1, /*stdout*\)
 Trace:FileStat:  < 0
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_getFileSize(/*stdin*\)
-Trace:FileStat:  > UTIL_stat(/*stdin*\)
+Trace:FileStat:  > UTIL_stat(-1, /*stdin*\)
 Trace:FileStat:  < 0
 Trace:FileStat: < -1

diff --git a/tests/cli-tests/file-stat/decompress-file-to-file.sh.stderr.exact b/tests/cli-tests/file-stat/decompress-file-to-file.sh.stderr.exact
index d264c63e4185d139be775d0378ecec5f601dc998..1d872fb7dc77244b01b9961599051de4af3c211e 100644 (file)
--- a/tests/cli-tests/file-stat/decompress-file-to-file.sh.stderr.exact
+++ b/tests/cli-tests/file-stat/decompress-file-to-file.sh.stderr.exact
@@ -5,29 +5,29 @@ Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isConsole(2)
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isDirectory(file.zst)
-Trace:FileStat:  > UTIL_stat(file.zst)
+Trace:FileStat:  > UTIL_stat(-1, file.zst)
 Trace:FileStat:  < 1
 Trace:FileStat:  > UTIL_isDirectoryStat()
 Trace:FileStat:  < 0
 Trace:FileStat: < 0
-Trace:FileStat: > UTIL_stat(file.zst)
+Trace:FileStat: > UTIL_stat(-1, file.zst)
 Trace:FileStat: < 1
 Trace:FileStat: > UTIL_isSameFile(file.zst, file)
-Trace:FileStat:  > UTIL_stat(file.zst)
+Trace:FileStat:  > UTIL_stat(-1, file.zst)
 Trace:FileStat:  < 1
-Trace:FileStat:  > UTIL_stat(file)
+Trace:FileStat:  > UTIL_stat(-1, file)
 Trace:FileStat:  < 0
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isRegularFile(file)
-Trace:FileStat:  > UTIL_stat(file)
+Trace:FileStat:  > UTIL_stat(-1, file)
 Trace:FileStat:  < 0
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isRegularFile(file)
-Trace:FileStat:  > UTIL_stat(file)
+Trace:FileStat:  > UTIL_stat(-1, file)
 Trace:FileStat:  < 1
 Trace:FileStat: < 1
-Trace:FileStat: > UTIL_setFileStat(file)
-Trace:FileStat:  > UTIL_stat(file)
+Trace:FileStat: > UTIL_setFileStat(-1, file)
+Trace:FileStat:  > UTIL_stat(-1, file)
 Trace:FileStat:  < 1
 Trace:FileStat:  > UTIL_utime(file)
 Trace:FileStat:  < 0

diff --git a/tests/cli-tests/file-stat/decompress-file-to-stdout.sh.stderr.exact b/tests/cli-tests/file-stat/decompress-file-to-stdout.sh.stderr.exact
index 7fe6dda1503b4e9bea9c17737bf990a77e75d908..8b60063c5fce08a813d3ededb6fd34e336d96b11 100644 (file)
--- a/tests/cli-tests/file-stat/decompress-file-to-stdout.sh.stderr.exact
+++ b/tests/cli-tests/file-stat/decompress-file-to-stdout.sh.stderr.exact
@@ -5,14 +5,14 @@ Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isConsole(2)
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isDirectory(file.zst)
-Trace:FileStat:  > UTIL_stat(file.zst)
+Trace:FileStat:  > UTIL_stat(-1, file.zst)
 Trace:FileStat:  < 1
 Trace:FileStat:  > UTIL_isDirectoryStat()
 Trace:FileStat:  < 0
 Trace:FileStat: < 0
-Trace:FileStat: > UTIL_stat(file.zst)
+Trace:FileStat: > UTIL_stat(-1, file.zst)
 Trace:FileStat: < 1
 Trace:FileStat: > UTIL_isRegularFile(/*stdout*\)
-Trace:FileStat:  > UTIL_stat(/*stdout*\)
+Trace:FileStat:  > UTIL_stat(-1, /*stdout*\)
 Trace:FileStat:  < 0
 Trace:FileStat: < 0

diff --git a/tests/cli-tests/file-stat/decompress-stdin-to-file.sh.stderr.exact b/tests/cli-tests/file-stat/decompress-stdin-to-file.sh.stderr.exact
index 749fd395a58467658aac9cef9cea2d0746b222e3..716a7ac7b3473160d1d885b6f6613e8e6695aecc 100644 (file)
--- a/tests/cli-tests/file-stat/decompress-stdin-to-file.sh.stderr.exact
+++ b/tests/cli-tests/file-stat/decompress-stdin-to-file.sh.stderr.exact
@@ -3,18 +3,18 @@ Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isConsole(2)
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isDirectory(/*stdin*\)
-Trace:FileStat:  > UTIL_stat(/*stdin*\)
+Trace:FileStat:  > UTIL_stat(-1, /*stdin*\)
 Trace:FileStat:  < 0
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isSameFile(/*stdin*\, file)
-Trace:FileStat:  > UTIL_stat(/*stdin*\)
+Trace:FileStat:  > UTIL_stat(-1, /*stdin*\)
 Trace:FileStat:  < 0
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isRegularFile(file)
-Trace:FileStat:  > UTIL_stat(file)
+Trace:FileStat:  > UTIL_stat(-1, file)
 Trace:FileStat:  < 0
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isRegularFile(file)
-Trace:FileStat:  > UTIL_stat(file)
+Trace:FileStat:  > UTIL_stat(-1, file)
 Trace:FileStat:  < 1
 Trace:FileStat: < 1

diff --git a/tests/cli-tests/file-stat/decompress-stdin-to-stdout.sh.stderr.exact b/tests/cli-tests/file-stat/decompress-stdin-to-stdout.sh.stderr.exact
index e36cb9d05f88e11563d4171d2f55ddd4f25c774e..2f6412059fc2e01da674d2746caceb9d39ada240 100644 (file)
--- a/tests/cli-tests/file-stat/decompress-stdin-to-stdout.sh.stderr.exact
+++ b/tests/cli-tests/file-stat/decompress-stdin-to-stdout.sh.stderr.exact
@@ -5,10 +5,10 @@ Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isConsole(2)
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isDirectory(/*stdin*\)
-Trace:FileStat:  > UTIL_stat(/*stdin*\)
+Trace:FileStat:  > UTIL_stat(-1, /*stdin*\)
 Trace:FileStat:  < 0
 Trace:FileStat: < 0
 Trace:FileStat: > UTIL_isRegularFile(/*stdout*\)
-Trace:FileStat:  > UTIL_stat(/*stdout*\)
+Trace:FileStat:  > UTIL_stat(-1, /*stdout*\)
 Trace:FileStat:  < 0
 Trace:FileStat: < 0