From a49417b5af0cf5669e6a97c0a24028db53276b2d Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 2 Dec 2019 14:28:18 -0800 Subject: [PATCH] fix recent issue combining -r with empty list of input files This would resize the table of input filenames to zero, delivering an empty table, to which it was no longer possible to add stdin. --- programs/util.c | 20 ++++++++++++++------ programs/zstdcli.c | 28 ++++++++++++++++------------ tests/playTests.sh | 16 +++++++++++----- 3 files changed, 41 insertions(+), 23 deletions(-) diff --git a/programs/util.c b/programs/util.c index 41ac1379..0fc95512 100644 --- a/programs/util.c +++ b/programs/util.c @@ -362,18 +362,24 @@ UTIL_createFileNamesTable_fromFileName(const char* inputFileName) } } -FileNamesTable* -UTIL_assembleFileNamesTable(const char** filenames, size_t tableSize, char* buf) +static FileNamesTable* +UTIL_assembleFileNamesTable2(const char** filenames, size_t tableSize, size_t tableCapacity, char* buf) { FileNamesTable* const table = (FileNamesTable*) malloc(sizeof(*table)); CONTROL(table != NULL); table->fileNames = filenames; table->buf = buf; table->tableSize = tableSize; - table->tableCapacity = tableSize; + table->tableCapacity = tableCapacity; return table; } +FileNamesTable* +UTIL_assembleFileNamesTable(const char** filenames, size_t tableSize, char* buf) +{ + return UTIL_assembleFileNamesTable2(filenames, tableSize, tableSize, buf); +} + void UTIL_freeFileNamesTable(FileNamesTable* table) { if (table==NULL) return; @@ -652,10 +658,11 @@ UTIL_createExpandedFNT(const char** inputNames, size_t nbIfns, int followLinks) if (buf == NULL) return NULL; } } } - if (nbFiles == 0) { free(buf); return NULL; } + /* note : even if nbFiles==0, function returns a valid, though empty, FileNamesTable* object */ { size_t ifnNb, pos; - const char** const fileNamesTable = (const char**)malloc((nbFiles + 1) * sizeof(*fileNamesTable)); + size_t const fntCapacity = nbFiles + 1; /* minimum 1, allows adding one reference, typically stdin */ + const char** const fileNamesTable = (const char**)malloc(fntCapacity * sizeof(*fileNamesTable)); if (!fileNamesTable) { free(buf); return NULL; } for (ifnNb = 0, pos = 0; ifnNb < nbFiles; ifnNb++) { @@ -663,7 +670,7 @@ UTIL_createExpandedFNT(const char** inputNames, size_t nbIfns, int followLinks) if (buf + pos > bufend) { free(buf); free((void*)fileNamesTable); return NULL; } pos += strlen(fileNamesTable[ifnNb]) + 1; } - return UTIL_assembleFileNamesTable(fileNamesTable, nbFiles, buf); + return UTIL_assembleFileNamesTable2(fileNamesTable, nbFiles, fntCapacity, buf); } } @@ -671,6 +678,7 @@ UTIL_createExpandedFNT(const char** inputNames, size_t nbIfns, int followLinks) void UTIL_expandFNT(FileNamesTable** fnt, int followLinks) { FileNamesTable* const newFNT = UTIL_createExpandedFNT((*fnt)->fileNames, (*fnt)->tableSize, followLinks); + CONTROL(newFNT != NULL); UTIL_freeFileNamesTable(*fnt); *fnt = newFNT; } diff --git a/programs/zstdcli.c b/programs/zstdcli.c index cf2f3682..15fcd8a5 100644 --- a/programs/zstdcli.c +++ b/programs/zstdcli.c @@ -27,10 +27,12 @@ **************************************/ #include "platform.h" /* IS_CONSOLE, PLATFORM_POSIX_VERSION */ #include "util.h" /* UTIL_HAS_CREATEFILELIST, UTIL_createFileList */ -#include /* fprintf(), stdin, stdout, stderr */ #include /* getenv */ #include /* strcmp, strlen */ +#include /* fprintf(), stdin, stdout, stderr */ #include /* errno */ +#include /* assert */ + #include "fileio.h" /* stdinmark, stdoutmark, ZSTD_EXTENSION */ #ifndef ZSTD_NOBENCH # include "benchzstd.h" /* BMK_benchFiles */ @@ -508,11 +510,13 @@ static void printVersion(void) /* Environment variables for parameter setting */ #define ENV_CLEVEL "ZSTD_CLEVEL" -/* functions that pick up environment variables */ +/* pick up environment variables + * requirement : g_displayOut must be set */ static int init_cLevel(void) { const char* const env = getenv(ENV_CLEVEL); - if (env) { - const char *ptr = env; + assert(g_displayOut == stderr); /* to print error messages */ + if (env != NULL) { + const char* ptr = env; int sign = 1; if (*ptr == '-') { sign = -1; @@ -524,14 +528,13 @@ static int init_cLevel(void) { if ((*ptr>='0') && (*ptr<='9')) { unsigned absLevel; if (readU32FromCharChecked(&ptr, &absLevel)) { - DISPLAYLEVEL(2, "Ignore environment variable setting %s=%s: numeric value too large\n", ENV_CLEVEL, env); + DISPLAYLEVEL(2, "Ignore environment variable setting %s=%s: numeric value too large \n", ENV_CLEVEL, env); return ZSTDCLI_CLEVEL_DEFAULT; } else if (*ptr == 0) { return sign * (int)absLevel; - } - } + } } - DISPLAYLEVEL(2, "Ignore environment variable setting %s=%s: not a valid integer value\n", ENV_CLEVEL, env); + DISPLAYLEVEL(2, "Ignore environment variable setting %s=%s: not a valid integer value \n", ENV_CLEVEL, env); } return ZSTDCLI_CLEVEL_DEFAULT; @@ -543,14 +546,14 @@ typedef enum { zom_compress, zom_decompress, zom_test, zom_bench, zom_train, zom #ifdef ZSTD_NOCOMPRESS /* symbols from compression library are not defined and should not be invoked */ -# define MINCLEVEL -50 +# define MINCLEVEL -99 # define MAXCLEVEL 22 #else # define MINCLEVEL ZSTD_minCLevel() # define MAXCLEVEL ZSTD_maxCLevel() #endif -int main(int argCount, const char* argv[]) +int main(int const argCount, const char* argv[]) { int argNb, followLinks = 0, @@ -582,7 +585,7 @@ int main(int argCount, const char* argv[]) zstd_operation_mode operation = zom_compress; ZSTD_compressionParameters compressionParams; int cLevel; - int cLevelLast = -1000000000; + int cLevelLast = MINCLEVEL - 1; /* lower than minimum */ unsigned recursive = 0; unsigned memLimit = 0; FileNamesTable* filenames = UTIL_allocateFileNamesTable((size_t)argCount); /* argCount >= 1 */ @@ -613,9 +616,10 @@ int main(int argCount, const char* argv[]) /* init */ (void)recursive; (void)cLevelLast; /* not used when ZSTD_NOBENCH set */ (void)memLimit; /* not used when ZSTD_NODECOMPRESS set */ + assert(argCount >= 1); if ((filenames==NULL) || (file_of_names==NULL)) { DISPLAY("zstd: allocation error \n"); exit(1); } g_displayOut = stderr; - cLevel = init_cLevel(); + cLevel = init_cLevel(); /* must be done after setting g_displayOut, since some error message might be printed */ programName = lastNameFromPath(programName); #ifdef ZSTD_MULTITHREAD nbWorkers = 1; diff --git a/tests/playTests.sh b/tests/playTests.sh index 84cb2d52..4886aac2 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -142,7 +142,7 @@ ZSTD_CLEVEL=- $ZSTD -f tmp # malformed env var, warn and revert to default set ZSTD_CLEVEL=a $ZSTD -f tmp # malformed env var, warn and revert to default setting ZSTD_CLEVEL=+a $ZSTD -f tmp # malformed env var, warn and revert to default setting ZSTD_CLEVEL=3a7 $ZSTD -f tmp # malformed env var, warn and revert to default setting -ZSTD_CLEVEL=50000000000 $ZSTD -f tmp # numeric value too large, warn and revert to default setting +ZSTD_CLEVEL=50000000000 $ZSTD -f tmp # numeric value too large, warn and revert to default setting println "test : override ZSTD_CLEVEL with command line option" ZSTD_CLEVEL=12 $ZSTD --fast=3 -f tmp # overridden by command line option println "test : compress to stdout" @@ -239,18 +239,24 @@ if [[ $file2timestamp -ge $file1timestamp ]]; then else println "Test is not successful" fi -#File Extension check. +# File Extension check. ./datagen $size > precompressedFilterTestDir/input.zstbar $ZSTD --exclude-compressed --long --rm -r precompressedFilterTestDir -#ZSTD should compress input.zstbar +# ZSTD should compress input.zstbar test -f precompressedFilterTestDir/input.zstbar.zst -#Check without the --exclude-compressed flag +# Check without the --exclude-compressed flag $ZSTD --long --rm -r precompressedFilterTestDir -#Files should get compressed again without the --exclude-compressed flag. +# Files should get compressed again without the --exclude-compressed flag. test -f precompressedFilterTestDir/input.5.zst.zst test -f precompressedFilterTestDir/input.6.zst.zst println "Test completed" + +println "\n===> recursive mode test " +# combination of -r with empty list of input file +$ZSTD -c -r < tmp > tmp.zst + + println "\n===> file removal" $ZSTD -f --rm tmp test ! -f tmp # tmp should no longer be present