From b11bea56a53d300f43b500be55890dbd01393d60 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Wed, 5 Aug 2020 00:09:29 -0400 Subject: [PATCH 01/14] Introduce Dedicated Helper to Call stat() --- programs/util.c | 11 +++++++++++ programs/util.h | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/programs/util.c b/programs/util.c index 2493a4adc..c172150d9 100644 --- a/programs/util.c +++ b/programs/util.c @@ -99,6 +99,17 @@ int g_utilDisplayLevel; * Functions ***************************************/ +int UTIL_stat(const char* filename, stat_t* statbuf) +{ +#if defined(_MSC_VER) + return !_stat64(filename, statbuf); +#elif defined(__MINGW32__) && defined (__MSVCRT__) + return !_stati64(filename, statbuf); +#else + return !stat(filename, statbuf); +#endif +} + int UTIL_fileExist(const char* filename) { stat_t statbuf; diff --git a/programs/util.h b/programs/util.h index 580266e71..0b58dfbbf 100644 --- a/programs/util.h +++ b/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,6 +115,11 @@ extern int g_utilDisplayLevel; #define STRDUP(s) strdup(s) #endif +/** + * 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); int UTIL_fileExist(const char* filename); int UTIL_isRegularFile(const char* infilename); int UTIL_isDirectory(const char* infilename); From 69cb9e77986b5ede06ced25f4bcf1a68e8307959 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Wed, 5 Aug 2020 00:21:41 -0400 Subject: [PATCH 02/14] Use New Stat Helper --- programs/util.c | 42 +++++++++++++----------------------------- 1 file changed, 13 insertions(+), 29 deletions(-) diff --git a/programs/util.c b/programs/util.c index c172150d9..3df3d3d02 100644 --- a/programs/util.c +++ b/programs/util.c @@ -113,12 +113,7 @@ int UTIL_stat(const char* filename, stat_t* statbuf) int UTIL_fileExist(const char* filename) { stat_t statbuf; -#if defined(_MSC_VER) - int const stat_error = _stat64(filename, &statbuf); -#else - int const stat_error = stat(filename, &statbuf); -#endif - return !stat_error; + return UTIL_stat(filename, &statbuf); } int UTIL_isRegularFile(const char* infilename) @@ -129,27 +124,22 @@ int UTIL_isRegularFile(const char* infilename) int UTIL_getFileStat(const char* infilename, stat_t *statbuf) { - int r; + const int r = UTIL_stat(infilename, statbuf); #if defined(_MSC_VER) - r = _stat64(infilename, statbuf); - if (r || !(statbuf->st_mode & S_IFREG)) return 0; /* No good... */ + return r && (statbuf->st_mode & S_IFREG); #else - r = stat(infilename, statbuf); - if (r || !S_ISREG(statbuf->st_mode)) return 0; /* No good... */ + return r && S_ISREG(statbuf->st_mode); #endif - return 1; } int UTIL_getDirectoryStat(const char* infilename, stat_t *statbuf) { + const int r = UTIL_stat(infilename, statbuf); #if defined(_MSC_VER) - int const r = _stat64(infilename, statbuf); - if (!r && (statbuf->st_mode & _S_IFDIR)) return 1; + return r && (statbuf->st_mode & _S_IFDIR); #else - int const r = stat(infilename, statbuf); - if (!r && S_ISDIR(statbuf->st_mode)) return 1; + return r && S_ISDIR(statbuf->st_mode); #endif - return 0; } /* like chmod, but avoid changing permission of /dev/null */ @@ -254,23 +244,17 @@ int UTIL_isLink(const char* infilename) U64 UTIL_getFileSize(const char* infilename) { + stat_t statbuf; + if (!UTIL_stat(infilename, &statbuf)) return UTIL_FILESIZE_UNKNOWN; if (!UTIL_isRegularFile(infilename)) return UTIL_FILESIZE_UNKNOWN; - { int r; #if defined(_MSC_VER) - struct __stat64 statbuf; - r = _stat64(infilename, &statbuf); - if (r || !(statbuf.st_mode & S_IFREG)) return UTIL_FILESIZE_UNKNOWN; + if (!(statbuf.st_mode & S_IFREG)) return UTIL_FILESIZE_UNKNOWN; #elif defined(__MINGW32__) && defined (__MSVCRT__) - struct _stati64 statbuf; - r = _stati64(infilename, &statbuf); - if (r || !(statbuf.st_mode & S_IFREG)) return UTIL_FILESIZE_UNKNOWN; + if (!(statbuf.st_mode & S_IFREG)) return UTIL_FILESIZE_UNKNOWN; #else - struct stat statbuf; - r = stat(infilename, &statbuf); - if (r || !S_ISREG(statbuf.st_mode)) return UTIL_FILESIZE_UNKNOWN; + if (!S_ISREG(statbuf.st_mode)) return UTIL_FILESIZE_UNKNOWN; #endif - return (U64)statbuf.st_size; - } + return (U64)statbuf.st_size; } From 5fbc6addb6a2a271d9164631f2fa81f24158a9c1 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Wed, 5 Aug 2020 00:31:48 -0400 Subject: [PATCH 03/14] Additionally Convert UTIL_getFileStat() Calls to UTIL_stat() Where Appropriate --- programs/util.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/programs/util.c b/programs/util.c index 3df3d3d02..56a0124ff 100644 --- a/programs/util.c +++ b/programs/util.c @@ -209,8 +209,8 @@ int UTIL_isSameFile(const char* fName1, const char* fName2) #else { stat_t file1Stat; stat_t file2Stat; - return UTIL_getFileStat(fName1, &file1Stat) - && UTIL_getFileStat(fName2, &file2Stat) + return UTIL_stat(fName1, &file1Stat) + && UTIL_stat(fName2, &file2Stat) && (file1Stat.st_dev == file2Stat.st_dev) && (file1Stat.st_ino == file2Stat.st_ino); } @@ -223,8 +223,8 @@ 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 (!r && S_ISFIFO(statbuf.st_mode)) return 1; + int const r = UTIL_stat(infilename, &statbuf); + if (r && S_ISFIFO(statbuf.st_mode)) return 1; #endif (void)infilename; return 0; From 1a1003f996e069a4c0edb09b88aabcce76263ef6 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Wed, 5 Aug 2020 00:35:21 -0400 Subject: [PATCH 04/14] Mark stat_t Arg to UTIL_setFileStat() const --- programs/util.c | 2 +- programs/util.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/util.c b/programs/util.c index 56a0124ff..785a37535 100644 --- a/programs/util.c +++ b/programs/util.c @@ -149,7 +149,7 @@ int UTIL_chmod(char const* filename, mode_t permissions) 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; diff --git a/programs/util.h b/programs/util.h index 0b58dfbbf..7618e2489 100644 --- a/programs/util.h +++ b/programs/util.h @@ -132,7 +132,7 @@ int UTIL_isFIFO(const char* infilename); 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_setFileStat(const char* filename, const 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); From b6e24bc4dc633d9596a06fc86e55dc3ff93b110e Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Wed, 5 Aug 2020 00:40:16 -0400 Subject: [PATCH 05/14] Rename UTIL_getFileStat() -> UTIL_statFile() and UTIL_getDirectoryStat() -> UTIL_statDir() I want to introduce versions of many of these functions that take pre- populated `stat_t` objects and use those rather than doing their own redundant `stat()` internally. These functions will have `...Stat()` suffixes. So this commit renames these existing functions into the active voice, to avoid confusion. --- programs/fileio.c | 4 ++-- programs/util.c | 10 +++++----- programs/util.h | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index b8183d89f..e818614b2 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -1478,7 +1478,7 @@ static int FIO_compressFilename_dstFile(FIO_prefs_t* const prefs, addHandler(dstFileName); if ( strcmp (srcFileName, stdinmark) - && UTIL_getFileStat(srcFileName, &statbuf)) + && UTIL_statFile(srcFileName, &statbuf)) transfer_permissions = 1; } @@ -2344,7 +2344,7 @@ 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_statFile(srcFileName, &statbuf) ) transfer_permissions = 1; } diff --git a/programs/util.c b/programs/util.c index 785a37535..3f91adeb5 100644 --- a/programs/util.c +++ b/programs/util.c @@ -119,10 +119,10 @@ int UTIL_fileExist(const char* filename) 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_statFile(infilename, &statbuf); /* Only need to know whether it is a regular file */ } -int UTIL_getFileStat(const char* infilename, stat_t *statbuf) +int UTIL_statFile(const char* infilename, stat_t *statbuf) { const int r = UTIL_stat(infilename, statbuf); #if defined(_MSC_VER) @@ -132,7 +132,7 @@ int UTIL_getFileStat(const char* infilename, stat_t *statbuf) #endif } -int UTIL_getDirectoryStat(const char* infilename, stat_t *statbuf) +int UTIL_statDir(const char* infilename, stat_t *statbuf) { const int r = UTIL_stat(infilename, statbuf); #if defined(_MSC_VER) @@ -190,7 +190,7 @@ int UTIL_setFileStat(const char *filename, const stat_t *statbuf) int UTIL_isDirectory(const char* infilename) { stat_t statbuf; - return UTIL_getDirectoryStat(infilename, &statbuf); + return UTIL_statDir(infilename, &statbuf); } int UTIL_compareStr(const void *p1, const void *p2) { @@ -647,7 +647,7 @@ static int isFileNameValidForMirroredOutput(const char *filename) static mode_t getDirMode(const char *dirName) { stat_t st; - int ret = UTIL_getDirectoryStat(dirName, &st); + int ret = UTIL_statDir(dirName, &st); if (!ret) { UTIL_DISPLAY("zstd: failed to get DIR stats %s: %s\n", dirName, strerror(errno)); return DIR_DEFAULT_MODE; diff --git a/programs/util.h b/programs/util.h index 7618e2489..ed3bd09ec 100644 --- a/programs/util.h +++ b/programs/util.h @@ -120,6 +120,8 @@ extern int g_utilDisplayLevel; * Returns success (1) or failure (0). */ int UTIL_stat(const char* filename, stat_t* statbuf); +int UTIL_statFile(const char* infilename, stat_t* statbuf); /* also check it's a file */ +int UTIL_statDir(const char* infilename, stat_t* statbuf); /* also check it's a directory */ int UTIL_fileExist(const char* filename); int UTIL_isRegularFile(const char* infilename); int UTIL_isDirectory(const char* infilename); @@ -131,9 +133,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, const 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); From 44fa052599d36b87c0e42219f05d0119d2726a5a Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Wed, 5 Aug 2020 01:00:06 -0400 Subject: [PATCH 06/14] Introduce Variants of Various UTIL Functions that Take Pre-Populated stat_t Structs Instead of calling `stat()`, these functions accept the result of a previous `stat()` call on the file in question, which will allow us to make multiple decisions around a file without redundant `stat()` calls. --- programs/util.c | 61 ++++++++++++++++++++++++++++++++++--------------- programs/util.h | 10 ++++++-- 2 files changed, 50 insertions(+), 21 deletions(-) diff --git a/programs/util.c b/programs/util.c index 3f91adeb5..c81587b47 100644 --- a/programs/util.c +++ b/programs/util.c @@ -122,24 +122,23 @@ int UTIL_isRegularFile(const char* infilename) return UTIL_statFile(infilename, &statbuf); /* Only need to know whether it is a regular file */ } +int UTIL_isRegularFileStat(const stat_t* statbuf) +{ +#if defined(_MSC_VER) + return (statbuf->st_mode & S_IFREG) != 0; +#else + return S_ISREG(statbuf->st_mode) != 0; +#endif +} + int UTIL_statFile(const char* infilename, stat_t *statbuf) { - const int r = UTIL_stat(infilename, statbuf); -#if defined(_MSC_VER) - return r && (statbuf->st_mode & S_IFREG); -#else - return r && S_ISREG(statbuf->st_mode); -#endif + return UTIL_stat(infilename, statbuf) && UTIL_isRegularFileStat(statbuf); } int UTIL_statDir(const char* infilename, stat_t *statbuf) { - const int r = UTIL_stat(infilename, statbuf); -#if defined(_MSC_VER) - return r && (statbuf->st_mode & _S_IFDIR); -#else - return r && S_ISDIR(statbuf->st_mode); -#endif + return UTIL_stat(infilename, statbuf) && UTIL_isDirectoryStat(statbuf); } /* like chmod, but avoid changing permission of /dev/null */ @@ -193,6 +192,15 @@ int UTIL_isDirectory(const char* infilename) return UTIL_statDir(infilename, &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) { return strcmp(* (char * const *) p1, * (char * const *) p2); } @@ -223,13 +231,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_stat(infilename, &statbuf); - if (r && S_ISFIFO(statbuf.st_mode)) return 1; + if (UTIL_stat(infilename, &statbuf) && UTIL_isFIFOStat(&statbuf)) 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 */ @@ -246,15 +264,20 @@ U64 UTIL_getFileSize(const char* infilename) { stat_t statbuf; if (!UTIL_stat(infilename, &statbuf)) return UTIL_FILESIZE_UNKNOWN; - if (!UTIL_isRegularFile(infilename)) 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) - if (!(statbuf.st_mode & S_IFREG)) return UTIL_FILESIZE_UNKNOWN; + if (!(statbuf->st_mode & S_IFREG)) return UTIL_FILESIZE_UNKNOWN; #elif defined(__MINGW32__) && defined (__MSVCRT__) - if (!(statbuf.st_mode & S_IFREG)) return UTIL_FILESIZE_UNKNOWN; + if (!(statbuf->st_mode & S_IFREG)) return UTIL_FILESIZE_UNKNOWN; #else - if (!S_ISREG(statbuf.st_mode)) return UTIL_FILESIZE_UNKNOWN; + if (!S_ISREG(statbuf->st_mode)) return UTIL_FILESIZE_UNKNOWN; #endif - return (U64)statbuf.st_size; + return (U64)statbuf->st_size; } diff --git a/programs/util.h b/programs/util.h index ed3bd09ec..0a3be31cb 100644 --- a/programs/util.h +++ b/programs/util.h @@ -120,18 +120,24 @@ extern int g_utilDisplayLevel; * Returns success (1) or failure (0). */ int UTIL_stat(const char* filename, stat_t* statbuf); -int UTIL_statFile(const char* infilename, stat_t* statbuf); /* also check it's a file */ -int UTIL_statDir(const char* infilename, stat_t* statbuf); /* also check it's a directory */ +/** Also checks that the target is a regular file. */ +int UTIL_statFile(const char* infilename, stat_t* statbuf); +/** Also checks that the target is a directory. */ +int UTIL_statDir(const char* infilename, stat_t* statbuf); int UTIL_fileExist(const char* filename); int UTIL_isRegularFile(const char* infilename); +int UTIL_isRegularFileStat(const stat_t* statbuf); /* same but takes existing statbuf */ int UTIL_isDirectory(const char* infilename); +int UTIL_isDirectoryStat(const stat_t* statbuf); /* same but takes existing statbuf */ int UTIL_isSameFile(const char* file1, const char* file2); int UTIL_isCompressedFile(const char* infilename, const char *extensionList[]); int UTIL_isLink(const char* infilename); int UTIL_isFIFO(const char* infilename); +int UTIL_isFIFOStat(const stat_t* statbuf); /* same but takes existing statbuf */ #define UTIL_FILESIZE_UNKNOWN ((U64)(-1)) U64 UTIL_getFileSize(const char* infilename); +U64 UTIL_getFileSizeStat(const stat_t* statbuf); U64 UTIL_getTotalFileSize(const char* const * fileNamesTable, unsigned nbFiles); int UTIL_setFileStat(const char* filename, const stat_t* statbuf); int UTIL_chmod(char const* filename, mode_t permissions); /*< like chmod, but avoid changing permission of /dev/null */ From 7238cca1a1f00b27570b721b852317dfcfbffa72 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Wed, 5 Aug 2020 01:08:34 -0400 Subject: [PATCH 07/14] Deduplicate Some Low-Hanging Fruit of Redundant Stat Calls --- programs/fileio.c | 7 ++++--- programs/util.c | 5 +++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index e818614b2..0633d39df 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -519,6 +519,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 +527,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) - && !UTIL_isFIFO(srcFileName) + if (!UTIL_isRegularFileStat(&statbuf) + && !UTIL_isFIFOStat(&statbuf) ) { DISPLAYLEVEL(1, "zstd: %s is not a regular file -- ignored \n", srcFileName); diff --git a/programs/util.c b/programs/util.c index c81587b47..69b93b8d9 100644 --- a/programs/util.c +++ b/programs/util.c @@ -354,11 +354,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 */ From 0a8aacb4db881e43b41e7043605fcf82f45836c1 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Wed, 5 Aug 2020 12:00:12 -0400 Subject: [PATCH 08/14] Use stat() to Check that File Should be chmod()-ed Rather than special-casing a check for `/dev/null`, this uses `stat()` to avoid `chmod()`-ing any non-regular file. I believe this is the desirable behavior. `UTIL_chmod()` is never called on directories at the moment, only output files. --- programs/fileio.c | 4 ++-- programs/util.c | 11 ++++++++--- programs/util.h | 7 ++++++- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 0633d39df..7d58a11e7 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -509,7 +509,7 @@ static int FIO_remove(const char* path) #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); + UTIL_chmod(path, NULL, _S_IWRITE); #endif return remove(path); } @@ -619,7 +619,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; } diff --git a/programs/util.c b/programs/util.c index 69b93b8d9..fd4d7a604 100644 --- a/programs/util.c +++ b/programs/util.c @@ -142,9 +142,14 @@ int UTIL_statDir(const char* infilename, stat_t *statbuf) } /* 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); } @@ -180,7 +185,7 @@ int UTIL_setFileStat(const char *filename, const 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, NULL, statbuf->st_mode & 07777); /* Copy file permissions */ errno = 0; return -res; /* number of errors is returned */ diff --git a/programs/util.h b/programs/util.h index 0a3be31cb..f75bca042 100644 --- a/programs/util.h +++ b/programs/util.h @@ -140,7 +140,12 @@ U64 UTIL_getFileSize(const char* infilename); U64 UTIL_getFileSizeStat(const stat_t* statbuf); U64 UTIL_getTotalFileSize(const char* const * fileNamesTable, unsigned nbFiles); int UTIL_setFileStat(const char* filename, const stat_t* statbuf); -int UTIL_chmod(char const* filename, mode_t permissions); /*< like chmod, but avoid changing permission of /dev/null */ +/** + * 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 that it should be modified. + */ +int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions); 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); From c1449143c5502367ca1277f01d28416880d37e9b Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Wed, 5 Aug 2020 12:10:42 -0400 Subject: [PATCH 09/14] Share stat() Calls in Uses of UTIL_chmod() --- programs/fileio.c | 13 ++++++++++--- programs/util.c | 5 +++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 7d58a11e7..e289f9abf 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -502,14 +502,21 @@ void FIO_setContentSize(FIO_prefs_t* const prefs, int value) * @result : Unlink `fileName`, even if it's read-only */ static int FIO_remove(const char* path) { - if (!UTIL_isRegularFile(path)) { - DISPLAYLEVEL(2, "zstd: Refusing to remove non-regular file %s \n", path); + stat_t statbuf; + 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, NULL, _S_IWRITE); + if (!(statbuf.mode & _S_IWRITE)) { + UTIL_chmod(path, &statbuf, _S_IWRITE); + } #endif return remove(path); } diff --git a/programs/util.c b/programs/util.c index fd4d7a604..9f9f45d2b 100644 --- a/programs/util.c +++ b/programs/util.c @@ -157,7 +157,8 @@ 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 */ @@ -185,7 +186,7 @@ int UTIL_setFileStat(const char *filename, const stat_t *statbuf) res += chown(filename, statbuf->st_uid, statbuf->st_gid); /* Copy ownership */ #endif - res += UTIL_chmod(filename, NULL, 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 */ From 76878697a4e4a902f2b4617b71b0ad6f2c71bf9e Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 10 Aug 2020 15:16:14 -0400 Subject: [PATCH 10/14] Re-Organize and Document Prototypes in util.h --- programs/util.h | 43 ++++++++++++++++++++++++++++++++----------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/programs/util.h b/programs/util.h index f75bca042..57cdc6058 100644 --- a/programs/util.h +++ b/programs/util.h @@ -124,28 +124,49 @@ int UTIL_stat(const char* filename, stat_t* statbuf); int UTIL_statFile(const char* infilename, stat_t* statbuf); /** Also checks that the target is a directory. */ int UTIL_statDir(const char* infilename, 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_fileExist(const char* filename); int UTIL_isRegularFile(const char* infilename); -int UTIL_isRegularFileStat(const stat_t* statbuf); /* same but takes existing statbuf */ int UTIL_isDirectory(const char* infilename); -int UTIL_isDirectoryStat(const stat_t* statbuf); /* same but takes existing statbuf */ int UTIL_isSameFile(const char* file1, const char* file2); int UTIL_isCompressedFile(const char* infilename, const char *extensionList[]); int UTIL_isLink(const char* infilename); int UTIL_isFIFO(const char* infilename); -int UTIL_isFIFOStat(const stat_t* statbuf); /* same but takes existing statbuf */ #define UTIL_FILESIZE_UNKNOWN ((U64)(-1)) U64 UTIL_getFileSize(const char* infilename); -U64 UTIL_getFileSizeStat(const stat_t* statbuf); U64 UTIL_getTotalFileSize(const char* const * fileNamesTable, unsigned nbFiles); -int UTIL_setFileStat(const char* filename, 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 that it should be modified. - */ -int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions); + 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); From 93dda988c85b6bbd688cba44a6c21233aa7e78ea Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 10 Aug 2020 15:22:29 -0400 Subject: [PATCH 11/14] Remove Unused Function UTIL_fileExist() --- programs/util.c | 6 ------ programs/util.h | 1 - 2 files changed, 7 deletions(-) diff --git a/programs/util.c b/programs/util.c index 9f9f45d2b..420d6aa70 100644 --- a/programs/util.c +++ b/programs/util.c @@ -110,12 +110,6 @@ int UTIL_stat(const char* filename, stat_t* statbuf) #endif } -int UTIL_fileExist(const char* filename) -{ - stat_t statbuf; - return UTIL_stat(filename, &statbuf); -} - int UTIL_isRegularFile(const char* infilename) { stat_t statbuf; diff --git a/programs/util.h b/programs/util.h index 57cdc6058..f903ccb13 100644 --- a/programs/util.h +++ b/programs/util.h @@ -155,7 +155,6 @@ int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions); * compute the needed information. */ -int UTIL_fileExist(const char* filename); int UTIL_isRegularFile(const char* infilename); int UTIL_isDirectory(const char* infilename); int UTIL_isSameFile(const char* file1, const char* file2); From 51ac0207afb1477a65c0b7a4ed407970cf63b243 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 10 Aug 2020 15:28:02 -0400 Subject: [PATCH 12/14] Remove UTIL_statFile() and UTIL_statDir(); Decompose Former Call-Sites --- programs/fileio.c | 6 ++++-- programs/util.c | 21 +++++++-------------- programs/util.h | 4 ---- 3 files changed, 11 insertions(+), 20 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index e289f9abf..b6d8fb5aa 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -1486,7 +1486,8 @@ static int FIO_compressFilename_dstFile(FIO_prefs_t* const prefs, addHandler(dstFileName); if ( strcmp (srcFileName, stdinmark) - && UTIL_statFile(srcFileName, &statbuf)) + && UTIL_stat(srcFileName, &statbuf) + && UTIL_isRegularFileStat(&statbuf) ) transfer_permissions = 1; } @@ -2352,7 +2353,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_statFile(srcFileName, &statbuf) ) + && UTIL_stat(srcFileName, &statbuf) + && UTIL_isRegularFileStat(&statbuf) ) transfer_permissions = 1; } diff --git a/programs/util.c b/programs/util.c index 420d6aa70..fc88ab35c 100644 --- a/programs/util.c +++ b/programs/util.c @@ -113,7 +113,7 @@ int UTIL_stat(const char* filename, stat_t* statbuf) int UTIL_isRegularFile(const char* infilename) { stat_t statbuf; - return UTIL_statFile(infilename, &statbuf); /* Only need to know whether it is a regular file */ + return UTIL_stat(infilename, &statbuf) && UTIL_isRegularFileStat(&statbuf); } int UTIL_isRegularFileStat(const stat_t* statbuf) @@ -125,16 +125,6 @@ int UTIL_isRegularFileStat(const stat_t* statbuf) #endif } -int UTIL_statFile(const char* infilename, stat_t *statbuf) -{ - return UTIL_stat(infilename, statbuf) && UTIL_isRegularFileStat(statbuf); -} - -int UTIL_statDir(const char* infilename, stat_t *statbuf) -{ - return UTIL_stat(infilename, statbuf) && UTIL_isDirectoryStat(statbuf); -} - /* like chmod, but avoid changing permission of /dev/null */ int UTIL_chmod(char const* filename, const stat_t* statbuf, mode_t permissions) { @@ -189,7 +179,7 @@ int UTIL_setFileStat(const char *filename, const stat_t *statbuf) int UTIL_isDirectory(const char* infilename) { stat_t statbuf; - return UTIL_statDir(infilename, &statbuf); + return UTIL_stat(infilename, &statbuf) && UTIL_isDirectoryStat(&statbuf); } int UTIL_isDirectoryStat(const stat_t* statbuf) @@ -671,11 +661,14 @@ static int isFileNameValidForMirroredOutput(const char *filename) static mode_t getDirMode(const char *dirName) { stat_t st; - int ret = UTIL_statDir(dirName, &st); - if (!ret) { + if (!UTIL_stat(dirName, &st)) { 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; } diff --git a/programs/util.h b/programs/util.h index f903ccb13..f8eee86ad 100644 --- a/programs/util.h +++ b/programs/util.h @@ -120,10 +120,6 @@ extern int g_utilDisplayLevel; * Returns success (1) or failure (0). */ int UTIL_stat(const char* filename, stat_t* statbuf); -/** Also checks that the target is a regular file. */ -int UTIL_statFile(const char* infilename, stat_t* statbuf); -/** Also checks that the target is a directory. */ -int UTIL_statDir(const char* infilename, stat_t* statbuf); /** * Instead of getting a file's stats, this updates them with the info in the From b02cdf63b0a14ce79d9129629f4eddf171fdadd9 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 10 Aug 2020 15:39:14 -0400 Subject: [PATCH 13/14] Clean Up Redundant Checks, Rename FIO_remove() -> FIO_removeFile() --- programs/fileio.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index b6d8fb5aa..2c0d2772d 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -498,9 +498,9 @@ 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) { stat_t statbuf; if (!UTIL_stat(path, &statbuf)) { @@ -616,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" ); @@ -1505,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 */ - } else if ( strcmp(dstFileName, stdoutmark) - && strcmp(dstFileName, nulmark) - && transfer_permissions) { + FIO_removeFile(dstFileName); /* remove compression artefact; note don't do anything special if remove() fails */ + } else if (transfer_permissions) { DISPLAYLEVEL(6, "FIO_compressFilename_dstFile: transferring permissions into dst: %s \n", dstFileName); UTIL_setFileStat(dstFileName, &statbuf); } else { @@ -1588,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; @@ -2370,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 */ - } else { /* operation success */ - if ( strcmp(dstFileName, stdoutmark) /* special case : don't chmod stdout */ - && 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 */ + FIO_removeFile(dstFileName); /* remove decompression artefact; note: don't do anything special if remove() fails */ + } else if ( transfer_permissions /* file permissions correctly extracted from src */ ) { + UTIL_setFileStat(dstFileName, &statbuf); /* transfer file permissions from src into dst */ } } @@ -2419,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; From 953f0a072ac02773fd16f6ab1df037670bc3ae74 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 10 Aug 2020 17:28:34 -0400 Subject: [PATCH 14/14] Fix MS Build --- programs/fileio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/fileio.c b/programs/fileio.c index 2c0d2772d..d5b8a7d14 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -514,7 +514,7 @@ static int FIO_removeFile(const char* path) #if defined(_WIN32) || defined(WIN32) /* windows doesn't allow remove read-only files, * so try to make it writable first */ - if (!(statbuf.mode & _S_IWRITE)) { + if (!(statbuf.st_mode & _S_IWRITE)) { UTIL_chmod(path, &statbuf, _S_IWRITE); } #endif