From 173ef9dea2aa9f1a9fd54b27949fd03ff9add197 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 19 Dec 2018 18:30:57 -0800 Subject: [PATCH 01/10] fixed : detection of non-existing file better error message with test --- programs/fileio.c | 26 +++++++++++++++++--------- programs/util.c | 11 ++++++++++- programs/util.h | 13 ++++++------- tests/playTests.sh | 3 ++- 4 files changed, 35 insertions(+), 18 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index d5cd3416..ee9d060b 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -388,6 +388,12 @@ static FILE* FIO_openSrcFile(const char* srcFileName) return stdin; } + if (!UTIL_fileExist(srcFileName)) { + DISPLAYLEVEL(1, "zstd: %s : No such file or directory (can't stat) -- ignored \n", + srcFileName); + return NULL; + } + if (!UTIL_isRegularFile(srcFileName)) { DISPLAYLEVEL(1, "zstd: %s is not a regular file -- ignored \n", srcFileName); @@ -803,26 +809,28 @@ FIO_compressLz4Frame(cRess_t* ress, /* Main Loop */ while (readSize>0) { - size_t outSize; - - /* Compress Block */ - outSize = LZ4F_compressUpdate(ctx, ress->dstBuffer, ress->dstBufferSize, ress->srcBuffer, readSize, NULL); + size_t const outSize = LZ4F_compressUpdate(ctx, + ress->dstBuffer, ress->dstBufferSize, + ress->srcBuffer, readSize, NULL); if (LZ4F_isError(outSize)) EXM_THROW(35, "zstd: %s: lz4 compression failed : %s", srcFileName, LZ4F_getErrorName(outSize)); outFileSize += outSize; - if (srcFileSize == UTIL_FILESIZE_UNKNOWN) + if (srcFileSize == UTIL_FILESIZE_UNKNOWN) { DISPLAYUPDATE(2, "\rRead : %u MB ==> %.2f%%", (U32)(inFileSize>>20), (double)outFileSize/inFileSize*100) - else + } else { DISPLAYUPDATE(2, "\rRead : %u / %u MB ==> %.2f%%", (U32)(inFileSize>>20), (U32)(srcFileSize>>20), (double)outFileSize/inFileSize*100); + } /* Write Block */ - { size_t const sizeCheck = fwrite(ress->dstBuffer, 1, outSize, ress->dstFile); - if (sizeCheck!=outSize) EXM_THROW(36, "Write error : %s", strerror(errno)); } + { size_t const sizeCheck = fwrite(ress->dstBuffer, 1, outSize, ress->dstFile); + if (sizeCheck != outSize) + EXM_THROW(36, "Write error : %s", strerror(errno)); + } /* Read next block */ readSize = fread(ress->srcBuffer, (size_t)1, (size_t)blockSize, ress->srcFile); @@ -837,7 +845,7 @@ FIO_compressLz4Frame(cRess_t* ress, srcFileName, LZ4F_getErrorName(headerSize)); { size_t const sizeCheck = fwrite(ress->dstBuffer, 1, headerSize, ress->dstFile); - if (sizeCheck!=headerSize) + if (sizeCheck != headerSize) EXM_THROW(39, "Write error : %s (cannot write end of stream)", strerror(errno)); } diff --git a/programs/util.c b/programs/util.c index 5c5e1960..c8140b7a 100644 --- a/programs/util.c +++ b/programs/util.c @@ -16,8 +16,18 @@ extern "C" { /*-**************************************** * Dependencies ******************************************/ +#include /* strncmp */ +#include + #include "util.h" +int UTIL_fileExist(const char* filename) +{ + stat_t statbuf; + int const stat_success = stat(filename, &statbuf); + return !stat_success; +} + int UTIL_isRegularFile(const char* infilename) { stat_t statbuf; @@ -651,4 +661,3 @@ int UTIL_countPhysicalCores(void) #if defined (__cplusplus) } #endif - diff --git a/programs/util.h b/programs/util.h index d61ff526..b97c4174 100644 --- a/programs/util.h +++ b/programs/util.h @@ -20,10 +20,9 @@ extern "C" { * Dependencies ******************************************/ #include "platform.h" /* PLATFORM_POSIX_VERSION, ZSTD_NANOSLEEP_SUPPORT, ZSTD_SETPRIORITY_SUPPORT */ -#include /* malloc */ +#include /* malloc, realloc, free */ #include /* size_t, ptrdiff_t */ #include /* fprintf */ -#include /* strncmp */ #include /* stat, utime */ #include /* stat, chmod */ #if defined(_MSC_VER) @@ -34,7 +33,6 @@ extern "C" { # include /* utime */ #endif #include /* clock_t, clock, CLOCKS_PER_SEC, nanosleep */ -#include #include "mem.h" /* U32, U64 */ @@ -163,10 +161,11 @@ void UTIL_waitForNextTick(void); #endif +int UTIL_fileExist(const char* filename); int UTIL_isRegularFile(const char* infilename); -int UTIL_setFileStat(const char *filename, stat_t *statbuf); +int UTIL_setFileStat(const char* filename, stat_t* statbuf); U32 UTIL_isDirectory(const char* infilename); -int UTIL_getFileStat(const char* infilename, stat_t *statbuf); +int UTIL_getFileStat(const char* infilename, stat_t* statbuf); U32 UTIL_isLink(const char* infilename); #define UTIL_FILESIZE_UNKNOWN ((U64)(-1)) @@ -178,7 +177,7 @@ U64 UTIL_getTotalFileSize(const char* const * const fileNamesTable, unsigned nbF * A modified version of realloc(). * If UTIL_realloc() fails the original block is freed. */ -UTIL_STATIC void *UTIL_realloc(void *ptr, size_t size) +UTIL_STATIC void* UTIL_realloc(void *ptr, size_t size) { void *newptr = realloc(ptr, size); if (newptr) return newptr; @@ -186,7 +185,7 @@ UTIL_STATIC void *UTIL_realloc(void *ptr, size_t size) return NULL; } -int UTIL_prepareFileList(const char *dirName, char** bufStart, size_t* pos, char** bufEnd, int followLinks); +int UTIL_prepareFileList(const char* dirName, char** bufStart, size_t* pos, char** bufEnd, int followLinks); #ifdef _WIN32 # define UTIL_HAS_CREATEFILELIST #elif defined(__linux__) || (PLATFORM_POSIX_VERSION >= 200112L) /* opendir, readdir require POSIX.1-2001 */ diff --git a/tests/playTests.sh b/tests/playTests.sh index b861391e..a86476ab 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -186,7 +186,8 @@ rm -f tmpro tmpro.zst $ECHO "test: overwrite input file (must fail)" $ZSTD tmp -fo tmp && die "zstd overwrote the input file" $ZSTD tmp.zst -dfo tmp.zst && die "zstd overwrote the input file" - +$ECHO "test: properly detect input file does not exist" +$ZSTD nothere 2>&1 | grep "o such file" $ECHO "test : file removal" $ZSTD -f --rm tmp From de4eb06a77b6232cd5509c4ed1138c70f6817678 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 19 Dec 2018 18:38:37 -0800 Subject: [PATCH 02/10] fixed 1 transitive include --- zlibWrapper/examples/zwrapbench.c | 1 + 1 file changed, 1 insertion(+) diff --git a/zlibWrapper/examples/zwrapbench.c b/zlibWrapper/examples/zwrapbench.c index d2d6073f..40e9ed55 100644 --- a/zlibWrapper/examples/zwrapbench.c +++ b/zlibWrapper/examples/zwrapbench.c @@ -17,6 +17,7 @@ #include /* fprintf, fopen, ftello64 */ #include /* clock_t, clock, CLOCKS_PER_SEC */ #include /* toupper */ +#include /* errno */ #include "mem.h" #define ZSTD_STATIC_LINKING_ONLY From 105fa953cbbd2ccded5c97225f52c6ae1b3cc3bf Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 20 Dec 2018 09:16:40 -0800 Subject: [PATCH 03/10] use strerror() to generate error message as suggested by @terrelln . also: - hopefully fixed Windows version - changed the test, so that it passes on non-english OS stdlib errors. --- programs/fileio.c | 31 ++++++++++++++++--------------- programs/util.c | 8 ++++++-- tests/playTests.sh | 2 +- 3 files changed, 23 insertions(+), 18 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index ee9d060b..7666b072 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -365,7 +365,7 @@ void FIO_setNoProgress(unsigned noProgress) { static int FIO_remove(const char* path) { if (!UTIL_isRegularFile(path)) { - DISPLAYLEVEL(2, "zstd: Refusing to remove non-regular file %s\n", path); + DISPLAYLEVEL(2, "zstd: Refusing to remove non-regular file %s \n", path); return 0; } #if defined(_WIN32) || defined(WIN32) @@ -383,14 +383,14 @@ static FILE* FIO_openSrcFile(const char* srcFileName) { assert(srcFileName != NULL); if (!strcmp (srcFileName, stdinmark)) { - DISPLAYLEVEL(4,"Using stdin for input\n"); + DISPLAYLEVEL(4,"Using stdin for input \n"); SET_BINARY_MODE(stdin); return stdin; } if (!UTIL_fileExist(srcFileName)) { - DISPLAYLEVEL(1, "zstd: %s : No such file or directory (can't stat) -- ignored \n", - srcFileName); + DISPLAYLEVEL(1, "zstd: can't stat %s : %s -- ignored \n", + srcFileName, strerror(errno)); return NULL; } @@ -414,7 +414,7 @@ static FILE* FIO_openDstFile(const char* srcFileName, const char* dstFileName) { assert(dstFileName != NULL); if (!strcmp (dstFileName, stdoutmark)) { - DISPLAYLEVEL(4,"Using stdout for output\n"); + DISPLAYLEVEL(4,"Using stdout for output \n"); SET_BINARY_MODE(stdout); if (g_sparseFileSupport==1) { g_sparseFileSupport = 0; @@ -425,11 +425,12 @@ static FILE* FIO_openDstFile(const char* srcFileName, const char* dstFileName) if (srcFileName != NULL) { stat_t srcStat; stat_t dstStat; - if (UTIL_getFileStat(srcFileName, &srcStat) && UTIL_getFileStat(dstFileName, &dstStat)) { - if (srcStat.st_dev == dstStat.st_dev && srcStat.st_ino == dstStat.st_ino) { - DISPLAYLEVEL(1, "zstd: Refusing to open a output file which will overwrite the input file \n"); - return NULL; - } + if ( UTIL_getFileStat(srcFileName, &srcStat) + && UTIL_getFileStat(dstFileName, &dstStat) + && srcStat.st_dev == dstStat.st_dev + && srcStat.st_ino == dstStat.st_ino ) { + DISPLAYLEVEL(1, "zstd: Refusing to open a output file which will overwrite the input file \n"); + return NULL; } } @@ -438,12 +439,12 @@ static FILE* FIO_openDstFile(const char* srcFileName, const char* dstFileName) } if (UTIL_isRegularFile(dstFileName)) { - FILE* fCheck; - if (!strcmp(dstFileName, nulmark)) { - EXM_THROW(40, "%s is unexpectedly a regular file", dstFileName); - } /* Check if destination file already exists */ - fCheck = fopen( dstFileName, "rb" ); + FILE* const fCheck = fopen( dstFileName, "rb" ); + if (!strcmp(dstFileName, nulmark)) { + EXM_THROW(40, "%s is unexpectedly categorized as a regular file", + dstFileName); + } if (fCheck != NULL) { /* dst file exists, authorization prompt */ fclose(fCheck); if (!g_overwrite) { diff --git a/programs/util.c b/programs/util.c index c8140b7a..4ef988c2 100644 --- a/programs/util.c +++ b/programs/util.c @@ -24,8 +24,12 @@ extern "C" { int UTIL_fileExist(const char* filename) { stat_t statbuf; - int const stat_success = stat(filename, &statbuf); - return !stat_success; +#if defined(_MSC_VER) + int const stat_error = _stat64(filename, &statbuf); +#else + int const stat_error = stat(filename, &statbuf); +#endif + return !stat_error; } int UTIL_isRegularFile(const char* infilename) diff --git a/tests/playTests.sh b/tests/playTests.sh index a86476ab..fc45ef5e 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -187,7 +187,7 @@ $ECHO "test: overwrite input file (must fail)" $ZSTD tmp -fo tmp && die "zstd overwrote the input file" $ZSTD tmp.zst -dfo tmp.zst && die "zstd overwrote the input file" $ECHO "test: properly detect input file does not exist" -$ZSTD nothere 2>&1 | grep "o such file" +$ZSTD nothere && die "zstd hasn't detected that input file does not exist" $ECHO "test : file removal" $ZSTD -f --rm tmp From 72dbf1bcd04fe429f00e7435255a16470764871c Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 20 Dec 2018 12:27:12 -0800 Subject: [PATCH 04/10] removed strncpy() from `util.c` as Visual surprisingly complains about their usage. Replaced by memcpy() --- programs/fileio.c | 34 ++++++++++++++++++---------------- programs/util.c | 31 ++++++++++++++++++++----------- tests/playTests.sh | 6 +++--- 3 files changed, 41 insertions(+), 30 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 7666b072..d4aea159 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -270,9 +270,9 @@ void FIO_addAbortHandler() static FIO_compressionType_t g_compressionType = FIO_zstdCompression; void FIO_setCompressionType(FIO_compressionType_t compressionType) { g_compressionType = compressionType; } static U32 g_overwrite = 0; -void FIO_overwriteMode(void) { g_overwrite=1; } -static U32 g_sparseFileSupport = 1; /* 0: no sparse allowed; 1: auto (file yes, stdout no); 2: force sparse */ -void FIO_setSparseWrite(unsigned sparse) { g_sparseFileSupport=sparse; } +void FIO_overwriteMode(void) { g_overwrite = 1; } +static U32 g_sparseFileSupport = ZSTD_SPARSE_DEFAULT; /* 0: no sparse allowed; 1: auto (file yes, stdout no); 2: force sparse */ +void FIO_setSparseWrite(unsigned sparse) { g_sparseFileSupport = sparse; } static U32 g_dictIDFlag = 1; void FIO_setDictIDFlag(unsigned dictIDFlag) { g_dictIDFlag = dictIDFlag; } static U32 g_checksumFlag = 1; @@ -416,23 +416,23 @@ static FILE* FIO_openDstFile(const char* srcFileName, const char* dstFileName) if (!strcmp (dstFileName, stdoutmark)) { DISPLAYLEVEL(4,"Using stdout for output \n"); SET_BINARY_MODE(stdout); - if (g_sparseFileSupport==1) { + if (g_sparseFileSupport == 1) { g_sparseFileSupport = 0; DISPLAYLEVEL(4, "Sparse File Support is automatically disabled on stdout ; try --sparse \n"); } return stdout; } + if (srcFileName != NULL) { stat_t srcStat; stat_t dstStat; if ( UTIL_getFileStat(srcFileName, &srcStat) - && UTIL_getFileStat(dstFileName, &dstStat) - && srcStat.st_dev == dstStat.st_dev - && srcStat.st_ino == dstStat.st_ino ) { - DISPLAYLEVEL(1, "zstd: Refusing to open a output file which will overwrite the input file \n"); - return NULL; - } - } + && UTIL_getFileStat(dstFileName, &dstStat) ) { + if ( srcStat.st_dev == dstStat.st_dev + && srcStat.st_ino == dstStat.st_ino ) { + DISPLAYLEVEL(1, "zstd: Refusing to open a output file which will overwrite the input file \n"); + return NULL; + } } } if (g_sparseFileSupport == 1) { g_sparseFileSupport = ZSTD_SPARSE_DEFAULT; @@ -1440,12 +1440,14 @@ static unsigned FIO_fwriteSparse(FILE* file, const void* buffer, size_t bufferSi static void FIO_fwriteSparseEnd(FILE* file, unsigned storedSkips) { - if (storedSkips-->0) { /* implies g_sparseFileSupport>0 */ - int const seekResult = LONG_SEEK(file, storedSkips, SEEK_CUR); - if (seekResult != 0) EXM_THROW(69, "Final skip error (sparse file)"); + if (storedSkips>0) { + assert(g_sparseFileSupport > 0); /* storedSkips>0 implies sparse support is enabled */ + if (LONG_SEEK(file, storedSkips-1, SEEK_CUR) != 0) + EXM_THROW(69, "Final skip error (sparse file support)"); + /* last zero must be explicitly written, + * so that skipped ones get implicitly translated as zero by FS */ { const char lastZeroByte[1] = { 0 }; - size_t const sizeCheck = fwrite(lastZeroByte, 1, 1, file); - if (sizeCheck != 1) + if (fwrite(lastZeroByte, 1, 1, file) != 1) EXM_THROW(69, "Write error : cannot write last zero"); } } } diff --git a/programs/util.c b/programs/util.c index 4ef988c2..00fc4fe2 100644 --- a/programs/util.c +++ b/programs/util.c @@ -18,6 +18,7 @@ extern "C" { ******************************************/ #include /* strncmp */ #include +#include #include "util.h" @@ -175,21 +176,23 @@ int UTIL_prepareFileList(const char *dirName, char** bufStart, size_t* pos, char pathLength = dirLength+1+fnameLength; path[pathLength] = 0; if (cFile.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { - if (strcmp (cFile.cFileName, "..") == 0 || - strcmp (cFile.cFileName, ".") == 0) continue; - - nbFiles += UTIL_prepareFileList(path, bufStart, pos, bufEnd, followLinks); /* Recursively call "UTIL_prepareFileList" with the new path. */ + if ( strcmp (cFile.cFileName, "..") == 0 + || strcmp (cFile.cFileName, ".") == 0 ) + continue; + /* Recursively call "UTIL_prepareFileList" with the new path. */ + nbFiles += UTIL_prepareFileList(path, bufStart, pos, bufEnd, followLinks); if (*bufStart == NULL) { free(path); FindClose(hFile); return 0; } - } - else if ((cFile.dwFileAttributes & FILE_ATTRIBUTE_NORMAL) || (cFile.dwFileAttributes & FILE_ATTRIBUTE_ARCHIVE) || (cFile.dwFileAttributes & FILE_ATTRIBUTE_COMPRESSED)) { + } else if ( (cFile.dwFileAttributes & FILE_ATTRIBUTE_NORMAL) + || (cFile.dwFileAttributes & FILE_ATTRIBUTE_ARCHIVE) + || (cFile.dwFileAttributes & FILE_ATTRIBUTE_COMPRESSED) ) { if (*bufStart + *pos + pathLength >= *bufEnd) { - ptrdiff_t newListSize = (*bufEnd - *bufStart) + LIST_SIZE_INCREASE; + ptrdiff_t const newListSize = (*bufEnd - *bufStart) + LIST_SIZE_INCREASE; *bufStart = (char*)UTIL_realloc(*bufStart, newListSize); - *bufEnd = *bufStart + newListSize; if (*bufStart == NULL) { free(path); FindClose(hFile); return 0; } + *bufEnd = *bufStart + newListSize; } if (*bufStart + *pos + pathLength < *bufEnd) { - strncpy(*bufStart + *pos, path, *bufEnd - (*bufStart + *pos)); + memcpy(*bufStart + *pos, path, pathLength+1 /* include final \0 */); *pos += pathLength + 1; nbFiles++; } @@ -246,7 +249,7 @@ int UTIL_prepareFileList(const char *dirName, char** bufStart, size_t* pos, char if (*bufStart == NULL) { free(path); closedir(dir); return 0; } } if (*bufStart + *pos + pathLength < *bufEnd) { - strncpy(*bufStart + *pos, path, *bufEnd - (*bufStart + *pos)); + memcpy(*bufStart + *pos, path, pathLength + 1); /* with final \0 */ *pos += pathLength + 1; nbFiles++; } @@ -304,7 +307,7 @@ UTIL_createFileList(const char **inputNames, unsigned inputNamesNb, if (!buf) return NULL; } if (buf + pos + len < bufend) { - strncpy(buf + pos, inputNames[i], bufend - (buf + pos)); + memcpy(buf+pos, inputNames[i], len+1); /* with final \0 */ pos += len + 1; nbFiles++; } @@ -336,6 +339,7 @@ UTIL_createFileList(const char **inputNames, unsigned inputNamesNb, ******************************************/ int g_utilDisplayLevel; + /*-**************************************** * Time functions ******************************************/ @@ -354,6 +358,7 @@ U64 UTIL_getSpanTimeMicro(UTIL_time_t clockStart, UTIL_time_t clockEnd) } return 1000000ULL*(clockEnd.QuadPart - clockStart.QuadPart)/ticksPerSecond.QuadPart; } + U64 UTIL_getSpanTimeNano(UTIL_time_t clockStart, UTIL_time_t clockEnd) { static LARGE_INTEGER ticksPerSecond; @@ -367,7 +372,9 @@ U64 UTIL_getSpanTimeNano(UTIL_time_t clockStart, UTIL_time_t clockEnd) } #elif defined(__APPLE__) && defined(__MACH__) + UTIL_time_t UTIL_getTime(void) { return mach_absolute_time(); } + U64 UTIL_getSpanTimeMicro(UTIL_time_t clockStart, UTIL_time_t clockEnd) { static mach_timebase_info_data_t rate; @@ -436,11 +443,13 @@ U64 UTIL_getSpanTimeNano(UTIL_time_t begin, UTIL_time_t end) } #else /* relies on standard C (note : clock_t measurements can be wrong when using multi-threading) */ + typedef clock_t UTIL_time_t; #define UTIL_TIME_INITIALIZER 0 UTIL_time_t UTIL_getTime(void) { return clock(); } U64 UTIL_getSpanTimeMicro(UTIL_time_t clockStart, UTIL_time_t clockEnd) { return 1000000ULL * (clockEnd - clockStart) / CLOCKS_PER_SEC; } U64 UTIL_getSpanTimeNano(UTIL_time_t clockStart, UTIL_time_t clockEnd) { return 1000000000ULL * (clockEnd - clockStart) / CLOCKS_PER_SEC; } + #endif /* returns time span in microseconds */ diff --git a/tests/playTests.sh b/tests/playTests.sh index fc45ef5e..94a70458 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -184,9 +184,9 @@ $ZSTD tmpro -c --no-progress | $ZSTD -d -o "$INTOVOID" --no-progress $ZSTD tmpro -cv --no-progress | $ZSTD -dv -o "$INTOVOID" --no-progress rm -f tmpro tmpro.zst $ECHO "test: overwrite input file (must fail)" -$ZSTD tmp -fo tmp && die "zstd overwrote the input file" -$ZSTD tmp.zst -dfo tmp.zst && die "zstd overwrote the input file" -$ECHO "test: properly detect input file does not exist" +$ZSTD tmp -fo tmp && die "zstd compression overwrote the input file" +$ZSTD tmp.zst -dfo tmp.zst && die "zstd decompression overwrote the input file" +$ECHO "test: detect that input file does not exist" $ZSTD nothere && die "zstd hasn't detected that input file does not exist" $ECHO "test : file removal" From 65a441a8f08c1a7a89b565b9239d60fc6df1032f Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 20 Dec 2018 14:02:50 -0800 Subject: [PATCH 05/10] fixed stdlib implementation of time functions generated redefinitions --- programs/util.c | 2 -- programs/util.h | 8 ++++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/programs/util.c b/programs/util.c index 00fc4fe2..4a369c05 100644 --- a/programs/util.c +++ b/programs/util.c @@ -444,8 +444,6 @@ U64 UTIL_getSpanTimeNano(UTIL_time_t begin, UTIL_time_t end) #else /* relies on standard C (note : clock_t measurements can be wrong when using multi-threading) */ -typedef clock_t UTIL_time_t; -#define UTIL_TIME_INITIALIZER 0 UTIL_time_t UTIL_getTime(void) { return clock(); } U64 UTIL_getSpanTimeMicro(UTIL_time_t clockStart, UTIL_time_t clockEnd) { return 1000000ULL * (clockEnd - clockStart) / CLOCKS_PER_SEC; } U64 UTIL_getSpanTimeNano(UTIL_time_t clockStart, UTIL_time_t clockEnd) { return 1000000000ULL * (clockEnd - clockStart) / CLOCKS_PER_SEC; } diff --git a/programs/util.h b/programs/util.h index b97c4174..f78bcbe1 100644 --- a/programs/util.h +++ b/programs/util.h @@ -116,12 +116,16 @@ extern int g_utilDisplayLevel; * Time functions ******************************************/ #if defined(_WIN32) /* Windows */ + #define UTIL_TIME_INITIALIZER { { 0, 0 } } typedef LARGE_INTEGER UTIL_time_t; + #elif defined(__APPLE__) && defined(__MACH__) + #include #define UTIL_TIME_INITIALIZER 0 typedef U64 UTIL_time_t; + #elif (PLATFORM_POSIX_VERSION >= 200112L) \ && (defined(__UCLIBC__) \ || (defined(__GLIBC__) \ @@ -133,10 +137,14 @@ extern int g_utilDisplayLevel; typedef struct timespec UTIL_time_t; UTIL_time_t UTIL_getSpanTime(UTIL_time_t begin, UTIL_time_t end); + #else /* relies on standard C (note : clock_t measurements can be wrong when using multi-threading) */ + typedef clock_t UTIL_time_t; #define UTIL_TIME_INITIALIZER 0 + #endif + UTIL_time_t UTIL_getTime(void); U64 UTIL_getSpanTimeMicro(UTIL_time_t clockStart, UTIL_time_t clockEnd); U64 UTIL_getSpanTimeNano(UTIL_time_t clockStart, UTIL_time_t clockEnd); From ffba1424067b04c9f6a8d75ae2786e0665f4c9c8 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 20 Dec 2018 14:30:30 -0800 Subject: [PATCH 06/10] fixed file identity detection in 32-bit mode also : some library decided to use `index` as a global variable declared in standard header shadowing the ones used in fastcover.c :( --- lib/dictBuilder/fastcover.c | 8 ++++---- programs/util.c | 3 +-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/dictBuilder/fastcover.c b/lib/dictBuilder/fastcover.c index dfee4574..cf8eb50a 100644 --- a/lib/dictBuilder/fastcover.c +++ b/lib/dictBuilder/fastcover.c @@ -159,15 +159,15 @@ static COVER_segment_t FASTCOVER_selectSegment(const FASTCOVER_ctx_t *ctx, */ while (activeSegment.end < end) { /* Get hash value of current dmer */ - const size_t index = FASTCOVER_hashPtrToIndex(ctx->samples + activeSegment.end, f, d); + const size_t idx = FASTCOVER_hashPtrToIndex(ctx->samples + activeSegment.end, f, d); /* Add frequency of this index to score if this is the first occurence of index in active segment */ - if (segmentFreqs[index] == 0) { - activeSegment.score += freqs[index]; + if (segmentFreqs[idx] == 0) { + activeSegment.score += freqs[idx]; } /* Increment end of segment and segmentFreqs*/ activeSegment.end += 1; - segmentFreqs[index] += 1; + segmentFreqs[idx] += 1; /* If the window is now too large, drop the first position */ if (activeSegment.end - activeSegment.begin == dmersInK + 1) { /* Get hash value of the dmer to be eliminated from active segment */ diff --git a/programs/util.c b/programs/util.c index 4a369c05..34634318 100644 --- a/programs/util.c +++ b/programs/util.c @@ -16,11 +16,10 @@ extern "C" { /*-**************************************** * Dependencies ******************************************/ -#include /* strncmp */ +#include "util.h" /* note : ensure that platform.h is included first ! */ #include #include -#include "util.h" int UTIL_fileExist(const char* filename) { From 0ed8ee4a371a11b91a68677cdff7a7efee79fe57 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 20 Dec 2018 14:46:23 -0800 Subject: [PATCH 07/10] fixed wrong assert condition --- programs/fileio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/fileio.c b/programs/fileio.c index d4aea159..4070fa9c 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -2334,7 +2334,7 @@ FIO_listFile(fileInfo_t* total, const char* inFileName, int displayLevel) } displayInfo(inFileName, &info, displayLevel); *total = FIO_addFInfo(*total, info); - assert(error>=0 || error<=1); + assert(error == info_success || error == info_frame_error); return error; } } From e129174d1df03c1405e16d96819051341fd4e3bb Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 20 Dec 2018 14:54:05 -0800 Subject: [PATCH 08/10] fixed shadowing of variable time some standard lib do define `time` as a global variable shadowing local declarations ... --- tests/paramgrill.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/paramgrill.c b/tests/paramgrill.c index 7c52a92e..9f8c894e 100644 --- a/tests/paramgrill.c +++ b/tests/paramgrill.c @@ -12,7 +12,7 @@ /*-************************************ * Dependencies **************************************/ -#include "util.h" /* Compiler options, UTIL_GetFileSize */ +#include "util.h" /* Ensure platform.h is compiled first; also : compiler options, UTIL_GetFileSize */ #include /* malloc */ #include /* fprintf, fopen, ftello64 */ #include /* strcmp */ @@ -24,7 +24,6 @@ #include "zstd.h" #include "datagen.h" #include "xxhash.h" -#include "util.h" #include "benchfn.h" #include "benchzstd.h" #include "zstd_errors.h" @@ -879,12 +878,12 @@ BMK_printWinner(FILE* f, const int cLevel, const BMK_benchResult_t result, const if(TIMED) { const U64 mn_in_ns = 60ULL * TIMELOOP_NANOSEC; - const U64 time = UTIL_clockSpanNano(g_time); - const U64 minutes = time / mn_in_ns; + const U64 time_ns = UTIL_clockSpanNano(g_time); + const U64 minutes = time_ns / mn_in_ns; fprintf(f, "%1lu:%2lu:%05.2f - ", (unsigned long) minutes / 60, (unsigned long) minutes % 60, - (double)(time - (minutes * mn_in_ns)) / TIMELOOP_NANOSEC ); + (double)(time_ns - (minutes * mn_in_ns)) / TIMELOOP_NANOSEC ); } fprintf(f, "/* %s */ ", lvlstr); From 95784c654c5fc42e000677e727ac840fcf06bc6d Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 20 Dec 2018 14:56:44 -0800 Subject: [PATCH 09/10] fixed shadowing of stat variable some standard lib declares a `stat` variable at global scope shadowing local declarations .... --- lib/compress/zstd_compress_internal.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index 8111fb22..798dad14 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -751,10 +751,10 @@ MEM_STATIC double ZSTD_fWeight(U32 rawStat) { U32 const fp_accuracy = 8; U32 const fp_multiplier = (1 << fp_accuracy); - U32 const stat = rawStat + 1; - U32 const hb = ZSTD_highbit32(stat); + U32 const newStat = rawStat + 1; + U32 const hb = ZSTD_highbit32(newStat); U32 const BWeight = hb * fp_multiplier; - U32 const FWeight = (stat << fp_accuracy) >> hb; + U32 const FWeight = (newStat << fp_accuracy) >> hb; U32 const weight = BWeight + FWeight; assert(hb + fp_accuracy < 31); return (double)weight / fp_multiplier; From 068c9b89be470c0d01be9d85e3efa4762d09f4b0 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 20 Dec 2018 15:28:03 -0800 Subject: [PATCH 10/10] fixed zlibwrapper examples build --- zlibWrapper/Makefile | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/zlibWrapper/Makefile b/zlibWrapper/Makefile index ba69f199..0c19107b 100644 --- a/zlibWrapper/Makefile +++ b/zlibWrapper/Makefile @@ -71,25 +71,25 @@ valgrindTest: clean example fitblk example_zstd fitblk_zstd zwrapbench # $(CC) $(CFLAGS) $(CPPFLAGS) -c -o $@ $< minigzip: $(EXAMPLE_PATH)/minigzip.o $(ZLIBWRAPPER_PATH)/zstd_zlibwrapper.o $(GZFILES) $(ZSTDLIBRARY) - $(CC) $(LDFLAGS) $^ $(ZSTDLIBRARY) $(ZLIB_LIBRARY) -o $@ + $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $^ $(ZSTDLIBRARY) $(ZLIB_LIBRARY) -o $@ minigzip_zstd: $(EXAMPLE_PATH)/minigzip.o $(ZLIBWRAPPER_PATH)/zstdTurnedOn_zlibwrapper.o $(GZFILES) $(ZSTDLIBRARY) - $(CC) $(LDFLAGS) $^ $(ZSTDLIBRARY) $(ZLIB_LIBRARY) -o $@ + $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $^ $(ZSTDLIBRARY) $(ZLIB_LIBRARY) -o $@ example: $(EXAMPLE_PATH)/example.o $(ZLIBWRAPPER_PATH)/zstd_zlibwrapper.o $(GZFILES) $(ZSTDLIBRARY) - $(CC) $(LDFLAGS) $^ $(ZLIB_LIBRARY) -o $@ + $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $^ $(ZLIB_LIBRARY) -o $@ example_zstd: $(EXAMPLE_PATH)/example.o $(ZLIBWRAPPER_PATH)/zstdTurnedOn_zlibwrapper.o $(GZFILES) $(ZSTDLIBRARY) - $(CC) $(LDFLAGS) $^ $(ZLIB_LIBRARY) -o $@ + $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $^ $(ZLIB_LIBRARY) -o $@ fitblk: $(EXAMPLE_PATH)/fitblk.o $(ZLIBWRAPPER_PATH)/zstd_zlibwrapper.o $(ZSTDLIBRARY) - $(CC) $(LDFLAGS) $^ $(ZLIB_LIBRARY) -o $@ + $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $^ $(ZLIB_LIBRARY) -o $@ fitblk_zstd: $(EXAMPLE_PATH)/fitblk.o $(ZLIBWRAPPER_PATH)/zstdTurnedOn_zlibwrapper.o $(ZSTDLIBRARY) - $(CC) $(LDFLAGS) $^ $(ZLIB_LIBRARY) -o $@ + $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $^ $(ZLIB_LIBRARY) -o $@ zwrapbench: $(EXAMPLE_PATH)/zwrapbench.o $(ZLIBWRAPPER_PATH)/zstd_zlibwrapper.o $(PROGRAMS_PATH)/util.o $(PROGRAMS_PATH)/datagen.o $(ZSTDLIBRARY) - $(CC) $(LDFLAGS) $^ $(ZLIB_LIBRARY) -o $@ + $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) $^ $(ZLIB_LIBRARY) -o $@ $(ZLIBWRAPPER_PATH)/zstd_zlibwrapper.o: $(ZLIBWRAPPER_PATH)/zstd_zlibwrapper.c $(ZLIBWRAPPER_PATH)/zstd_zlibwrapper.h