Merge pull request #2262 from felixhandte/fewer-stat-syscalls

Share stat() Calls Where Convenient

programs/fileio.c

@@ -498,18 +498,25 @@ void FIO_setContentSize(FIO_prefs_t* const prefs, int value)
 /*-*************************************
 *  Functions
 ***************************************/
-/** FIO_remove() :
+/** FIO_removeFile() :
  * @result : Unlink `fileName`, even if it's read-only */
-static int FIO_remove(const char* path)
+static int FIO_removeFile(const char* path)
 {
-    if (!UTIL_isRegularFile(path)) {
+    stat_t statbuf;
-        DISPLAYLEVEL(2, "zstd: Refusing to remove non-regular file %s \n", path);
+    if (!UTIL_stat(path, &statbuf)) {
+        DISPLAYLEVEL(2, "zstd: Failed to stat %s while trying to remove it\n", path);
+        return 0;
+    }
+    if (!UTIL_isRegularFileStat(&statbuf)) {
+        DISPLAYLEVEL(2, "zstd: Refusing to remove non-regular file %s\n", path);
         return 0;
     }
 #if defined(_WIN32) || defined(WIN32)
     /* windows doesn't allow remove read-only files,
      * so try to make it writable first */
-    UTIL_chmod(path, _S_IWRITE);
+    if (!(statbuf.st_mode & _S_IWRITE)) {
+        UTIL_chmod(path, &statbuf, _S_IWRITE);
+    }
 #endif
     return remove(path);
 }
@@ -519,6 +526,7 @@ static int FIO_remove(const char* path)
  * @result : FILE* to `srcFileName`, or NULL if it fails */
 static FILE* FIO_openSrcFile(const char* srcFileName)
 {
+    stat_t statbuf;
     assert(srcFileName != NULL);
     if (!strcmp (srcFileName, stdinmark)) {
         DISPLAYLEVEL(4,"Using stdin for input \n");
@@ -526,14 +534,14 @@ static FILE* FIO_openSrcFile(const char* srcFileName)
         return stdin;
     }
 
-    if (!UTIL_fileExist(srcFileName)) {
+    if (!UTIL_stat(srcFileName, &statbuf)) {
         DISPLAYLEVEL(1, "zstd: can't stat %s : %s -- ignored \n",
                         srcFileName, strerror(errno));
         return NULL;
     }
 
-    if (!UTIL_isRegularFile(srcFileName)
+    if (!UTIL_isRegularFileStat(&statbuf)
-     && !UTIL_isFIFO(srcFileName)
+     && !UTIL_isFIFOStat(&statbuf)
     ) {
         DISPLAYLEVEL(1, "zstd: %s is not a regular file -- ignored \n",
                         srcFileName);
@@ -608,7 +616,7 @@ FIO_openDstFile(FIO_prefs_t* const prefs,
                     while ((ch!=EOF) && (ch!='\n')) ch = getchar();
             }   }
             /* need to unlink */
-            FIO_remove(dstFileName);
+            FIO_removeFile(dstFileName);
     }   }
 
     {   FILE* const f = fopen( dstFileName, "wb" );
@@ -618,7 +626,7 @@ FIO_openDstFile(FIO_prefs_t* const prefs,
                && strcmp (srcFileName, stdinmark)
                && strcmp(dstFileName, nulmark) ) {
             /* reduce rights on newly created dst file while compression is ongoing */
-            UTIL_chmod(dstFileName, 00600);
+            UTIL_chmod(dstFileName, NULL, 00600);
         }
         return f;
     }
@@ -1478,7 +1486,8 @@ static int FIO_compressFilename_dstFile(FIO_prefs_t* const prefs,
         addHandler(dstFileName);
 
         if ( strcmp (srcFileName, stdinmark)
-          && UTIL_getFileStat(srcFileName, &statbuf))
+          && UTIL_stat(srcFileName, &statbuf)
+          && UTIL_isRegularFileStat(&statbuf) )
             transfer_permissions = 1;
     }
 
@@ -1496,13 +1505,10 @@ static int FIO_compressFilename_dstFile(FIO_prefs_t* const prefs,
             result=1;
         }
         if ( (result != 0)  /* operation failure */
-          && strcmp(dstFileName, nulmark)     /* special case : don't remove() /dev/null */
           && strcmp(dstFileName, stdoutmark)  /* special case : don't remove() stdout */
           ) {
-            FIO_remove(dstFileName); /* remove compression artefact; note don't do anything special if remove() fails */
+            FIO_removeFile(dstFileName); /* remove compression artefact; note don't do anything special if remove() fails */
-        } else if ( strcmp(dstFileName, stdoutmark)
+        } else if (transfer_permissions) {
-                 && strcmp(dstFileName, nulmark)
-                 && transfer_permissions) {
             DISPLAYLEVEL(6, "FIO_compressFilename_dstFile: transferring permissions into dst: %s \n", dstFileName);
             UTIL_setFileStat(dstFileName, &statbuf);
         } else {
@@ -1579,7 +1585,7 @@ FIO_compressFilename_srcFile(FIO_prefs_t* const prefs,
          * delete both the source and destination files.
          */
         clearHandler();
-        if (FIO_remove(srcFileName))
+        if (FIO_removeFile(srcFileName))
             EXM_THROW(1, "zstd: %s: %s", srcFileName, strerror(errno));
     }
     return result;
@@ -2344,7 +2350,8 @@ static int FIO_decompressDstFile(FIO_prefs_t* const prefs,
         addHandler(dstFileName);
 
         if ( strcmp(srcFileName, stdinmark)   /* special case : don't transfer permissions from stdin */
-          && UTIL_getFileStat(srcFileName, &statbuf) )
+          && UTIL_stat(srcFileName, &statbuf)
+          && UTIL_isRegularFileStat(&statbuf) )
             transfer_permissions = 1;
     }
 
@@ -2360,15 +2367,11 @@ static int FIO_decompressDstFile(FIO_prefs_t* const prefs,
         }
 
         if ( (result != 0)  /* operation failure */
-          && strcmp(dstFileName, nulmark)     /* special case : don't remove() /dev/null (#316) */
           && strcmp(dstFileName, stdoutmark)  /* special case : don't remove() stdout */
           ) {
-            FIO_remove(dstFileName);  /* remove decompression artefact; note: don't do anything special if remove() fails */
+            FIO_removeFile(dstFileName);  /* remove decompression artefact; note: don't do anything special if remove() fails */
-        } else {  /* operation success */
+        } else if ( transfer_permissions /* file permissions correctly extracted from src */ ) {
-            if ( strcmp(dstFileName, stdoutmark) /* special case : don't chmod stdout */
+            UTIL_setFileStat(dstFileName, &statbuf);  /* transfer file permissions from src into dst */
-              && strcmp(dstFileName, nulmark)    /* special case : don't chmod /dev/null */
-              && transfer_permissions )          /* file permissions correctly extracted from src */
-                UTIL_setFileStat(dstFileName, &statbuf);  /* transfer file permissions from src into dst */
         }
     }
 
@@ -2409,7 +2412,7 @@ static int FIO_decompressSrcFile(FIO_prefs_t* const prefs, dRess_t ress, const c
          * delete both the source and destination files.
          */
         clearHandler();
-        if (FIO_remove(srcFileName)) {
+        if (FIO_removeFile(srcFileName)) {
             /* failed to remove src file */
             DISPLAYLEVEL(1, "zstd: %s: %s \n", srcFileName, strerror(errno));
             return 1;

programs/util.c

@@ -99,60 +99,50 @@ int g_utilDisplayLevel;
 *  Functions
 ***************************************/
 
-int UTIL_fileExist(const char* filename)
+int UTIL_stat(const char* filename, stat_t* statbuf)
 {
-    stat_t statbuf;
 #if defined(_MSC_VER)
-    int const stat_error = _stat64(filename, &statbuf);
+    return !_stat64(filename, statbuf);
+#elif defined(__MINGW32__) && defined (__MSVCRT__)
+    return !_stati64(filename, statbuf);
 #else
-    int const stat_error = stat(filename, &statbuf);
+    return !stat(filename, statbuf);
 #endif
-    return !stat_error;
 }
 
 int UTIL_isRegularFile(const char* infilename)
 {
     stat_t statbuf;
-    return UTIL_getFileStat(infilename, &statbuf); /* Only need to know whether it is a regular file */
+    return UTIL_stat(infilename, &statbuf) && UTIL_isRegularFileStat(&statbuf);
 }
 
-int UTIL_getFileStat(const char* infilename, stat_t *statbuf)
+int UTIL_isRegularFileStat(const stat_t* statbuf)
-{
-    int r;
-#if defined(_MSC_VER)
-    r = _stat64(infilename, statbuf);
-    if (r || !(statbuf->st_mode & S_IFREG)) return 0;   /* No good... */
-#else
-    r = stat(infilename, statbuf);
-    if (r || !S_ISREG(statbuf->st_mode)) return 0;   /* No good... */
-#endif
-    return 1;
-}
-
-int UTIL_getDirectoryStat(const char* infilename, stat_t *statbuf)
 {
 #if defined(_MSC_VER)
-    int const r = _stat64(infilename, statbuf);
+    return (statbuf->st_mode & S_IFREG) != 0;
-    if (!r && (statbuf->st_mode & _S_IFDIR)) return 1;
 #else
-    int const r = stat(infilename, statbuf);
+    return S_ISREG(statbuf->st_mode) != 0;
-    if (!r && S_ISDIR(statbuf->st_mode)) return 1;
 #endif
-    return 0;
 }
 
 /* like chmod, but avoid changing permission of /dev/null */
-int UTIL_chmod(char const* filename, mode_t permissions)
+int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions)
 {
-    if (!strcmp(filename, "/dev/null")) return 0;   /* pretend success, but don't change anything */
+    stat_t localStatBuf;
+    if (statbuf == NULL) {
+        if (!UTIL_stat(filename, &localStatBuf)) return 0;
+        statbuf = &localStatBuf;
+    }
+    if (!UTIL_isRegularFileStat(statbuf)) return 0; /* pretend success, but don't change anything */
     return chmod(filename, permissions);
 }
 
-int UTIL_setFileStat(const char *filename, stat_t *statbuf)
+int UTIL_setFileStat(const char *filename, const stat_t *statbuf)
 {
     int res = 0;
 
-    if (!UTIL_isRegularFile(filename))
+    stat_t curStatBuf;
+    if (!UTIL_stat(filename, &curStatBuf) || !UTIL_isRegularFileStat(&curStatBuf))
         return -1;
 
     /* set access and modification times */
@@ -180,7 +170,7 @@ int UTIL_setFileStat(const char *filename, stat_t *statbuf)
     res += chown(filename, statbuf->st_uid, statbuf->st_gid);  /* Copy ownership */
 #endif
 
-    res += UTIL_chmod(filename, statbuf->st_mode & 07777);  /* Copy file permissions */
+    res += UTIL_chmod(filename, &curStatBuf, statbuf->st_mode & 07777);  /* Copy file permissions */
 
     errno = 0;
     return -res; /* number of errors is returned */
@@ -189,7 +179,16 @@ int UTIL_setFileStat(const char *filename, stat_t *statbuf)
 int UTIL_isDirectory(const char* infilename)
 {
     stat_t statbuf;
-    return UTIL_getDirectoryStat(infilename, &statbuf);
+    return UTIL_stat(infilename, &statbuf) && UTIL_isDirectoryStat(&statbuf);
+}
+
+int UTIL_isDirectoryStat(const stat_t* statbuf)
+{
+#if defined(_MSC_VER)
+    return (statbuf->st_mode & _S_IFDIR) != 0;
+#else
+    return S_ISDIR(statbuf->st_mode) != 0;
+#endif
 }
 
 int UTIL_compareStr(const void *p1, const void *p2) {
@@ -208,8 +207,8 @@ int UTIL_isSameFile(const char* fName1, const char* fName2)
 #else
     {   stat_t file1Stat;
         stat_t file2Stat;
-        return UTIL_getFileStat(fName1, &file1Stat)
+        return UTIL_stat(fName1, &file1Stat)
-            && UTIL_getFileStat(fName2, &file2Stat)
+            && UTIL_stat(fName2, &file2Stat)
             && (file1Stat.st_dev == file2Stat.st_dev)
             && (file1Stat.st_ino == file2Stat.st_ino);
     }
@@ -222,13 +221,23 @@ int UTIL_isFIFO(const char* infilename)
 /* macro guards, as defined in : https://linux.die.net/man/2/lstat */
 #if PLATFORM_POSIX_VERSION >= 200112L
     stat_t statbuf;
-    int const r = UTIL_getFileStat(infilename, &statbuf);
+    if (UTIL_stat(infilename, &statbuf) && UTIL_isFIFOStat(&statbuf)) return 1;
-    if (!r && S_ISFIFO(statbuf.st_mode)) return 1;
 #endif
     (void)infilename;
     return 0;
 }
 
+/* UTIL_isFIFO : distinguish named pipes */
+int UTIL_isFIFOStat(const stat_t* statbuf)
+{
+/* macro guards, as defined in : https://linux.die.net/man/2/lstat */
+#if PLATFORM_POSIX_VERSION >= 200112L
+    if (S_ISFIFO(statbuf->st_mode)) return 1;
+#endif
+    (void)statbuf;
+    return 0;
+}
+
 int UTIL_isLink(const char* infilename)
 {
 /* macro guards, as defined in : https://linux.die.net/man/2/lstat */
@@ -243,23 +252,22 @@ int UTIL_isLink(const char* infilename)
 
 U64 UTIL_getFileSize(const char* infilename)
 {
-    if (!UTIL_isRegularFile(infilename)) return UTIL_FILESIZE_UNKNOWN;
+    stat_t statbuf;
-    {   int r;
+    if (!UTIL_stat(infilename, &statbuf)) return UTIL_FILESIZE_UNKNOWN;
+    return UTIL_getFileSizeStat(&statbuf);
+}
+
+U64 UTIL_getFileSizeStat(const stat_t* statbuf)
+{
+    if (!UTIL_isRegularFileStat(statbuf)) return UTIL_FILESIZE_UNKNOWN;
 #if defined(_MSC_VER)
-        struct __stat64 statbuf;
+    if (!(statbuf->st_mode & S_IFREG)) return UTIL_FILESIZE_UNKNOWN;
-        r = _stat64(infilename, &statbuf);
-        if (r || !(statbuf.st_mode & S_IFREG)) return UTIL_FILESIZE_UNKNOWN;
 #elif defined(__MINGW32__) && defined (__MSVCRT__)
-        struct _stati64 statbuf;
+    if (!(statbuf->st_mode & S_IFREG)) return UTIL_FILESIZE_UNKNOWN;
-        r = _stati64(infilename, &statbuf);
-        if (r || !(statbuf.st_mode & S_IFREG)) return UTIL_FILESIZE_UNKNOWN;
 #else
-        struct stat statbuf;
+    if (!S_ISREG(statbuf->st_mode)) return UTIL_FILESIZE_UNKNOWN;
-        r = stat(infilename, &statbuf);
-        if (r || !S_ISREG(statbuf.st_mode)) return UTIL_FILESIZE_UNKNOWN;
 #endif
-        return (U64)statbuf.st_size;
+    return (U64)statbuf->st_size;
-    }
 }
 
 
@@ -336,11 +344,12 @@ UTIL_createFileNamesTable_fromFileName(const char* inputFileName)
     char* buf;
     size_t bufSize;
     size_t pos = 0;
+    stat_t statbuf;
 
-    if (!UTIL_fileExist(inputFileName) || !UTIL_isRegularFile(inputFileName))
+    if (!UTIL_stat(inputFileName, &statbuf) || !UTIL_isRegularFileStat(&statbuf))
         return NULL;
 
-    {   U64 const inputFileSize = UTIL_getFileSize(inputFileName);
+    {   U64 const inputFileSize = UTIL_getFileSizeStat(&statbuf);
         if(inputFileSize > MAX_FILE_OF_FILE_NAMES_SIZE)
             return NULL;
         bufSize = (size_t)(inputFileSize + 1); /* (+1) to add '\0' at the end of last filename */
@@ -652,11 +661,14 @@ static int isFileNameValidForMirroredOutput(const char *filename)
 static mode_t getDirMode(const char *dirName)
 {
     stat_t st;
-    int ret = UTIL_getDirectoryStat(dirName, &st);
+    if (!UTIL_stat(dirName, &st)) {
-    if (!ret) {
         UTIL_DISPLAY("zstd: failed to get DIR stats %s: %s\n", dirName, strerror(errno));
         return DIR_DEFAULT_MODE;
     }
+    if (!UTIL_isDirectoryStat(&st)) {
+        UTIL_DISPLAY("zstd: expected directory: %s\n", dirName);
+        return DIR_DEFAULT_MODE;
+    }
     return st.st_mode;
 }
 

programs/util.h

@@ -100,6 +100,8 @@ extern int g_utilDisplayLevel;
 #if defined(_MSC_VER)
     typedef struct __stat64 stat_t;
     typedef int mode_t;
+#elif defined(__MINGW32__) && defined (__MSVCRT__)
+    typedef struct _stati64 stat_t;
 #else
     typedef struct stat stat_t;
 #endif
@@ -113,7 +115,42 @@ extern int g_utilDisplayLevel;
 #define STRDUP(s) strdup(s)
 #endif
 
-int UTIL_fileExist(const char* filename);
+/**
+ * Calls platform's equivalent of stat() on filename and writes info to statbuf.
+ * Returns success (1) or failure (0).
+ */
+int UTIL_stat(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.
+ */
+int UTIL_setFileStat(const char* filename, const stat_t* statbuf);
+
+/*
+ * These helpers operate on a pre-populated stat_t, i.e., the result of
+ * calling one of the above functions.
+ */
+
+int UTIL_isRegularFileStat(const stat_t* statbuf);
+int UTIL_isDirectoryStat(const stat_t* statbuf);
+int UTIL_isFIFOStat(const stat_t* statbuf);
+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.
+ */
+int UTIL_chmod(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
+ * functions will do a stat() call internally and then use that result to
+ * compute the needed information.
+ */
+
 int UTIL_isRegularFile(const char* infilename);
 int UTIL_isDirectory(const char* infilename);
 int UTIL_isSameFile(const char* file1, const char* file2);
@@ -124,10 +161,7 @@ int UTIL_isFIFO(const char* infilename);
 #define UTIL_FILESIZE_UNKNOWN  ((U64)(-1))
 U64 UTIL_getFileSize(const char* infilename);
 U64 UTIL_getTotalFileSize(const char* const * fileNamesTable, unsigned nbFiles);
-int UTIL_getFileStat(const char* infilename, stat_t* statbuf);
+
-int UTIL_setFileStat(const char* filename, stat_t* statbuf);
-int UTIL_getDirectoryStat(const char* infilename, stat_t* statbuf);
-int UTIL_chmod(char const* filename, mode_t permissions);   /*< like chmod, but avoid changing permission of /dev/null */
 int UTIL_compareStr(const void *p1, const void *p2);
 const char* UTIL_getFileExtension(const char* infilename);
 void  UTIL_mirrorSourceFilesDirectories(const char** fileNamesTable, unsigned int nbFiles, const char *outDirName);