From 165e52ce627169e285c06d1d7879623fb75fcadd Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 24 Mar 2025 20:42:52 -0700 Subject: [PATCH 1/9] first implementation supporting Process Substitution --- programs/fileio.c | 8 +- programs/util.c | 190 ++++++++++++++++++++++++++++------------------ programs/util.h | 1 + 3 files changed, 122 insertions(+), 77 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 3f4460594..644a86ef4 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -537,15 +537,13 @@ static int FIO_removeFile(const char* path) return remove(path); } -/** FIO_openSrcFile() : - * condition : `srcFileName` must be non-NULL. `prefs` may be NULL. - * @result : FILE* to `srcFileName`, or NULL if it fails */ static FILE* FIO_openSrcFile(const FIO_prefs_t* const prefs, const char* srcFileName, stat_t* statbuf) { int allowBlockDevices = prefs != NULL ? prefs->allowBlockDevices : 0; assert(srcFileName != NULL); assert(statbuf != NULL); - if (!strcmp (srcFileName, stdinmark)) { + + if (!strcmp(srcFileName, stdinmark)) { DISPLAYLEVEL(4,"Using stdin for input \n"); SET_BINARY_MODE(stdin); return stdin; @@ -557,8 +555,10 @@ static FILE* FIO_openSrcFile(const FIO_prefs_t* const prefs, const char* srcFile return NULL; } + /* Accept regular files, FIFOs, and process substitution file descriptors */ if (!UTIL_isRegularFileStat(statbuf) && !UTIL_isFIFOStat(statbuf) + && !UTIL_isFileDescriptorPipe(srcFileName) /* Process substitution support */ && !(allowBlockDevices && UTIL_isBlockDevStat(statbuf)) ) { DISPLAYLEVEL(1, "zstd: %s is not a regular file -- ignored \n", diff --git a/programs/util.c b/programs/util.c index 065a35855..506bdd09e 100644 --- a/programs/util.c +++ b/programs/util.c @@ -448,6 +448,26 @@ int UTIL_isFIFOStat(const stat_t* statbuf) return 0; } +/* process substitution */ +int UTIL_isFileDescriptorPipe(const char* filename) +{ + UTIL_TRACE_CALL("UTIL_isFileDescriptorPipe(%s)", filename); + /* Check if the filename is a /dev/fd/ path which indicates a file descriptor */ + if (filename[0] == '/' && strncmp(filename, "/dev/fd/", 8) == 0) { + UTIL_TRACE_RET(1); + return 1; + } + + /* Check for alternative process substitution formats on different systems */ + if (filename[0] == '/' && strncmp(filename, "/proc/self/fd/", 14) == 0) { + UTIL_TRACE_RET(1); + return 1; + } + + UTIL_TRACE_RET(0); + return 0; /* Not recognized as a file descriptor pipe */ +} + /* UTIL_isBlockDevStat : distinguish named pipes */ int UTIL_isBlockDevStat(const stat_t* statbuf) { @@ -614,95 +634,119 @@ U64 UTIL_getTotalFileSize(const char* const * fileNamesTable, unsigned nbFiles) } -/* condition : @file must be valid, and not have reached its end. - * @return : length of line written into @buf, ended with `\0` instead of '\n', - * or 0, if there is no new line */ -static size_t readLineFromFile(char* buf, size_t len, FILE* file) -{ - assert(!feof(file)); - if ( fgets(buf, (int) len, file) == NULL ) return 0; - { size_t linelen = strlen(buf); - if (strlen(buf)==0) return 0; - if (buf[linelen-1] == '\n') linelen--; - buf[linelen] = '\0'; - return linelen+1; - } -} - -/* Conditions : - * size of @inputFileName file must be < @dstCapacity - * @dst must be initialized - * @return : nb of lines - * or -1 if there's an error - */ -static int -readLinesFromFile(void* dst, size_t dstCapacity, - const char* inputFileName) -{ - int nbFiles = 0; - size_t pos = 0; - char* const buf = (char*)dst; - FILE* const inputFile = fopen(inputFileName, "r"); - - assert(dst != NULL); - - if(!inputFile) { - if (g_utilDisplayLevel >= 1) perror("zstd:util:readLinesFromFile"); - return -1; - } - - while ( !feof(inputFile) ) { - size_t const lineLength = readLineFromFile(buf+pos, dstCapacity-pos, inputFile); - if (lineLength == 0) break; - assert(pos + lineLength <= dstCapacity); /* '=' for inputFile not terminated with '\n' */ - pos += lineLength; - ++nbFiles; - } - - CONTROL( fclose(inputFile) == 0 ); - - return nbFiles; -} - /*Note: buf is not freed in case function successfully created table because filesTable->fileNames[0] = buf*/ FileNamesTable* UTIL_createFileNamesTable_fromFileName(const char* inputFileName) { size_t nbFiles = 0; - char* buf; - size_t bufSize; - stat_t statbuf; + char* buf = NULL; + size_t bufSize = 0; + size_t totalRead = 0; + size_t bytesRead = 0; - if (!UTIL_stat(inputFileName, &statbuf) || !UTIL_isRegularFileStat(&statbuf)) - return NULL; + /* Check if the input is a regular file or a file descriptor */ + { stat_t statbuf; + if (!UTIL_stat(inputFileName, &statbuf)) { + return NULL; + } - { U64 const inputFileSize = UTIL_getFileSizeStat(&statbuf); - if(inputFileSize > MAX_FILE_OF_FILE_NAMES_SIZE) + if (!UTIL_isRegularFileStat(&statbuf) && + !UTIL_isFIFOStat(&statbuf) && + !UTIL_isFileDescriptorPipe(inputFileName)) { return NULL; - bufSize = (size_t)(inputFileSize + 1); /* (+1) to add '\0' at the end of last filename */ - } - - buf = (char*) malloc(bufSize); - CONTROL( buf != NULL ); - - { int const ret_nbFiles = readLinesFromFile(buf, bufSize, inputFileName); - - if (ret_nbFiles <= 0) { - free(buf); - return NULL; } - nbFiles = (size_t)ret_nbFiles; } - { const char** filenamesTable = (const char**) malloc(nbFiles * sizeof(*filenamesTable)); - CONTROL(filenamesTable != NULL); + /* Open the input file */ + { FILE* const inFile = fopen(inputFileName, "rb"); + if (inFile == NULL) return NULL; - { size_t fnb, pos = 0; + /* Start with a reasonable buffer size */ + bufSize = 64 * 1024; + buf = (char*)malloc(bufSize); + if (buf == NULL) { + fclose(inFile); + return NULL; + } + + /* Read the file incrementally */ + while ((bytesRead = fread(buf + totalRead, 1, bufSize - totalRead - 1, inFile)) > 0) { + totalRead += bytesRead; + + /* If buffer is nearly full, expand it */ + if (bufSize - totalRead < 1024) { + size_t newBufSize; + if (bufSize >= MAX_FILE_OF_FILE_NAMES_SIZE) { + /* Too large, abort */ + free(buf); + fclose(inFile); + return NULL; + } + + newBufSize = bufSize * 2; + if (newBufSize > MAX_FILE_OF_FILE_NAMES_SIZE) + newBufSize = MAX_FILE_OF_FILE_NAMES_SIZE; + + { char* newBuf = (char*)realloc(buf, newBufSize); + if (newBuf == NULL) { + free(buf); + fclose(inFile); + return NULL; + } + + buf = newBuf; + } + bufSize = newBufSize; + } + } + + fclose(inFile); + } + + /* Add null terminator to the end */ + buf[totalRead] = '\0'; + + /* Count and process the lines */ + { + size_t lineCount = 0; + size_t i = 0; + + /* Convert newlines to null terminators and count lines */ + while (i < totalRead) { + if (buf[i] == '\n') { + buf[i] = '\0'; /* Replace newlines with null terminators */ + lineCount++; + } + i++; + } + + /* Count the last line if it doesn't end with a newline */ + if (totalRead > 0 && buf[totalRead-1] != '\0') { + lineCount++; + } + + nbFiles = lineCount; + } + + if (nbFiles == 0) { + free(buf); + return NULL; + } + + /* Create the file names table */ + { const char** filenamesTable = (const char**)malloc(nbFiles * sizeof(*filenamesTable)); + if (filenamesTable == NULL) { + free(buf); + return NULL; + } + + { + size_t fnb, pos = 0; for (fnb = 0; fnb < nbFiles; fnb++) { filenamesTable[fnb] = buf+pos; pos += strlen(buf+pos)+1; /* +1 for the finishing `\0` */ } - assert(pos <= bufSize); + assert(pos <= bufSize); } return UTIL_assembleFileNamesTable(filenamesTable, nbFiles, buf); diff --git a/programs/util.h b/programs/util.h index d768e7660..5fd5de40b 100644 --- a/programs/util.h +++ b/programs/util.h @@ -191,6 +191,7 @@ int UTIL_isSameFileStat(const char* file1, const char* file2, const stat_t* file int UTIL_isCompressedFile(const char* infilename, const char *extensionList[]); int UTIL_isLink(const char* infilename); int UTIL_isFIFO(const char* infilename); +int UTIL_isFileDescriptorPipe(const char* filename); /** * Returns with the given file descriptor is a console. From 7630870b4747bc90cba2aa90fea0df4a89bfabc3 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 24 Mar 2025 20:47:56 -0700 Subject: [PATCH 2/9] second implementation, better structured for improved maintenance --- programs/util.c | 192 ++++++++++++++++++++++++++---------------------- 1 file changed, 106 insertions(+), 86 deletions(-) diff --git a/programs/util.c b/programs/util.c index 506bdd09e..b6431b49f 100644 --- a/programs/util.c +++ b/programs/util.c @@ -151,7 +151,8 @@ int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg, /*-************************************* * Constants ***************************************/ -#define LIST_SIZE_INCREASE (8*1024) +#define KB * (1 << 10) +#define LIST_SIZE_INCREASE (8 KB) #define MAX_FILE_OF_FILE_NAMES_SIZE (1<<20)*50 @@ -634,125 +635,144 @@ U64 UTIL_getTotalFileSize(const char* const * fileNamesTable, unsigned nbFiles) } -/*Note: buf is not freed in case function successfully created table because filesTable->fileNames[0] = buf*/ -FileNamesTable* -UTIL_createFileNamesTable_fromFileName(const char* inputFileName) +/* Read the entire content of a file into a buffer with progressive resizing */ +static char* UTIL_readFileContent(FILE* inFile, size_t* totalReadPtr) { - size_t nbFiles = 0; - char* buf = NULL; - size_t bufSize = 0; + size_t bufSize = 64 KB; /* Start with a reasonable buffer size */ size_t totalRead = 0; size_t bytesRead = 0; + char* buf = (char*)malloc(bufSize); + if (buf == NULL) return NULL; - /* Check if the input is a regular file or a file descriptor */ - { stat_t statbuf; - if (!UTIL_stat(inputFileName, &statbuf)) { - return NULL; - } - if (!UTIL_isRegularFileStat(&statbuf) && - !UTIL_isFIFOStat(&statbuf) && - !UTIL_isFileDescriptorPipe(inputFileName)) { - return NULL; - } - } + /* Read the file incrementally */ + while ((bytesRead = fread(buf + totalRead, 1, bufSize - totalRead - 1, inFile)) > 0) { + totalRead += bytesRead; - /* Open the input file */ - { FILE* const inFile = fopen(inputFileName, "rb"); - if (inFile == NULL) return NULL; + /* If buffer is nearly full, expand it */ + if (bufSize - totalRead < 1 KB) { + if (bufSize >= MAX_FILE_OF_FILE_NAMES_SIZE) { + /* Too large, abort */ + free(buf); + return NULL; + } - /* Start with a reasonable buffer size */ - bufSize = 64 * 1024; - buf = (char*)malloc(bufSize); - if (buf == NULL) { - fclose(inFile); - return NULL; - } - - /* Read the file incrementally */ - while ((bytesRead = fread(buf + totalRead, 1, bufSize - totalRead - 1, inFile)) > 0) { - totalRead += bytesRead; - - /* If buffer is nearly full, expand it */ - if (bufSize - totalRead < 1024) { - size_t newBufSize; - if (bufSize >= MAX_FILE_OF_FILE_NAMES_SIZE) { - /* Too large, abort */ - free(buf); - fclose(inFile); - return NULL; - } - - newBufSize = bufSize * 2; + { size_t newBufSize = bufSize * 2; if (newBufSize > MAX_FILE_OF_FILE_NAMES_SIZE) newBufSize = MAX_FILE_OF_FILE_NAMES_SIZE; { char* newBuf = (char*)realloc(buf, newBufSize); if (newBuf == NULL) { free(buf); - fclose(inFile); return NULL; } buf = newBuf; - } - bufSize = newBufSize; - } - } - - fclose(inFile); + bufSize = newBufSize; + } } } } /* Add null terminator to the end */ buf[totalRead] = '\0'; + *totalReadPtr = totalRead; - /* Count and process the lines */ - { - size_t lineCount = 0; - size_t i = 0; + return buf; +} - /* Convert newlines to null terminators and count lines */ - while (i < totalRead) { - if (buf[i] == '\n') { - buf[i] = '\0'; /* Replace newlines with null terminators */ - lineCount++; - } - i++; - } +/* Process a buffer containing multiple lines and count the number of lines */ +static size_t UTIL_processLines(char* buffer, size_t bufferSize) +{ + size_t lineCount = 0; + size_t i = 0; - /* Count the last line if it doesn't end with a newline */ - if (totalRead > 0 && buf[totalRead-1] != '\0') { + /* Convert newlines to null terminators and count lines */ + while (i < bufferSize) { + if (buffer[i] == '\n') { + buffer[i] = '\0'; /* Replace newlines with null terminators */ lineCount++; } - - nbFiles = lineCount; + i++; } - if (nbFiles == 0) { - free(buf); + /* Count the last line if it doesn't end with a newline */ + if (bufferSize > 0 && (i == 0 || buffer[i-1] != '\0')) { + lineCount++; + } + + return lineCount; +} + +/* Create an array of pointers to the lines in a buffer */ +static const char** +UTIL_createLinePointers(char* buffer, size_t numLines, size_t bufferSize) +{ + size_t lineIndex = 0; + size_t pos = 0; + const char** linePointers = (const char**)malloc(numLines * sizeof(*linePointers)); + if (linePointers == NULL) return NULL; + + while (lineIndex < numLines && pos < bufferSize) { + linePointers[lineIndex++] = buffer + pos; + pos += strlen(buffer + pos) + 1; /* +1 for the finishing `\0` */ + } + + assert(pos <= bufferSize); + assert(lineIndex == numLines); + + return linePointers; +} + +FileNamesTable* +UTIL_createFileNamesTable_fromFileName(const char* inputFileName) +{ + stat_t statbuf; + FILE* inFile = NULL; + char* buffer = NULL; + const char** linePointers = NULL; + size_t numLines = 0; + size_t bufferSize = 0; + + /* Check if the input is a valid file */ + if (!UTIL_stat(inputFileName, &statbuf)) { return NULL; } - /* Create the file names table */ - { const char** filenamesTable = (const char**)malloc(nbFiles * sizeof(*filenamesTable)); - if (filenamesTable == NULL) { - free(buf); - return NULL; - } - - { - size_t fnb, pos = 0; - for (fnb = 0; fnb < nbFiles; fnb++) { - filenamesTable[fnb] = buf+pos; - pos += strlen(buf+pos)+1; /* +1 for the finishing `\0` */ - } - assert(pos <= bufSize); - } - - return UTIL_assembleFileNamesTable(filenamesTable, nbFiles, buf); + /* Check if the input is a supported type */ + if (!UTIL_isRegularFileStat(&statbuf) && + !UTIL_isFIFOStat(&statbuf) && + !UTIL_isFileDescriptorPipe(inputFileName)) { + return NULL; } + + /* Open the input file */ + inFile = fopen(inputFileName, "rb"); + if (inFile == NULL) return NULL; + + /* Read the file content */ + buffer = UTIL_readFileContent(inFile, &bufferSize); + fclose(inFile); + + if (buffer == NULL) return NULL; + + /* Process lines */ + numLines = UTIL_processLines(buffer, bufferSize); + if (numLines == 0) { + free(buffer); + return NULL; + } + + /* Create line pointers */ + linePointers = UTIL_createLinePointers(buffer, numLines, bufferSize); + if (linePointers == NULL) { + free(buffer); + return NULL; + } + + /* Create the final table */ + return UTIL_assembleFileNamesTable(linePointers, numLines, buffer); } + static FileNamesTable* UTIL_assembleFileNamesTable2(const char** filenames, size_t tableSize, size_t tableCapacity, char* buf) { From 76c2fdc7b76e1ab502dd058af9e58a22eeb382ad Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 24 Mar 2025 21:20:21 -0700 Subject: [PATCH 3/9] better naming and more narrow scope of local variables --- programs/util.c | 34 +++++++++++++++++----------------- programs/util.h | 4 ++-- programs/zstdcli.c | 2 +- tests/playTests.sh | 1 + 4 files changed, 21 insertions(+), 20 deletions(-) diff --git a/programs/util.c b/programs/util.c index b6431b49f..43f2b9408 100644 --- a/programs/util.c +++ b/programs/util.c @@ -723,34 +723,33 @@ UTIL_createLinePointers(char* buffer, size_t numLines, size_t bufferSize) } FileNamesTable* -UTIL_createFileNamesTable_fromFileName(const char* inputFileName) +UTIL_createFileNamesTable_fromFileList(const char* fileList) { stat_t statbuf; - FILE* inFile = NULL; char* buffer = NULL; - const char** linePointers = NULL; size_t numLines = 0; size_t bufferSize = 0; /* Check if the input is a valid file */ - if (!UTIL_stat(inputFileName, &statbuf)) { + if (!UTIL_stat(fileList, &statbuf)) { return NULL; } /* Check if the input is a supported type */ if (!UTIL_isRegularFileStat(&statbuf) && !UTIL_isFIFOStat(&statbuf) && - !UTIL_isFileDescriptorPipe(inputFileName)) { + !UTIL_isFileDescriptorPipe(fileList)) { return NULL; } /* Open the input file */ - inFile = fopen(inputFileName, "rb"); - if (inFile == NULL) return NULL; + { FILE* const inFile = fopen(fileList, "rb"); + if (inFile == NULL) return NULL; - /* Read the file content */ - buffer = UTIL_readFileContent(inFile, &bufferSize); - fclose(inFile); + /* Read the file content */ + buffer = UTIL_readFileContent(inFile, &bufferSize); + fclose(inFile); + } if (buffer == NULL) return NULL; @@ -762,14 +761,15 @@ UTIL_createFileNamesTable_fromFileName(const char* inputFileName) } /* Create line pointers */ - linePointers = UTIL_createLinePointers(buffer, numLines, bufferSize); - if (linePointers == NULL) { - free(buffer); - return NULL; - } + { const char** linePointers = UTIL_createLinePointers(buffer, numLines, bufferSize); + if (linePointers == NULL) { + free(buffer); + return NULL; + } - /* Create the final table */ - return UTIL_assembleFileNamesTable(linePointers, numLines, buffer); + /* Create the final table */ + return UTIL_assembleFileNamesTable(linePointers, numLines, buffer); + } } diff --git a/programs/util.h b/programs/util.h index 5fd5de40b..427bcf441 100644 --- a/programs/util.h +++ b/programs/util.h @@ -251,13 +251,13 @@ typedef struct size_t tableCapacity; } FileNamesTable; -/*! UTIL_createFileNamesTable_fromFileName() : +/*! UTIL_createFileNamesTable_fromFileList() : * read filenames from @inputFileName, and store them into returned object. * @return : a FileNamesTable*, or NULL in case of error (ex: @inputFileName doesn't exist). * Note: inputFileSize must be less than 50MB */ FileNamesTable* -UTIL_createFileNamesTable_fromFileName(const char* inputFileName); +UTIL_createFileNamesTable_fromFileList(const char* inputFileName); /*! UTIL_assembleFileNamesTable() : * This function takes ownership of its arguments, @filenames and @buf, diff --git a/programs/zstdcli.c b/programs/zstdcli.c index b9f961c16..fa7ea37b3 100644 --- a/programs/zstdcli.c +++ b/programs/zstdcli.c @@ -1379,7 +1379,7 @@ int main(int argCount, const char* argv[]) size_t const nbFileLists = file_of_names->tableSize; size_t flNb; for (flNb=0; flNb < nbFileLists; flNb++) { - FileNamesTable* const fnt = UTIL_createFileNamesTable_fromFileName(file_of_names->fileNames[flNb]); + FileNamesTable* const fnt = UTIL_createFileNamesTable_fromFileList(file_of_names->fileNames[flNb]); if (fnt==NULL) { DISPLAYLEVEL(1, "zstd: error reading %s \n", file_of_names->fileNames[flNb]); CLEAN_RETURN(1); diff --git a/tests/playTests.sh b/tests/playTests.sh index 486b395eb..d3ce39846 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -848,6 +848,7 @@ ls tmp* > tmpList zstd -f tmp1 --filelist=tmpList --filelist=tmpList tmp2 tmp3 # can trigger an overflow of internal file list rm -rf tmp* + println "\n===> --[no-]content-size tests" datagen > tmp_contentsize From 94cfa0b5a064b91a61f8b49148ea857e042b51a1 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 24 Mar 2025 21:31:51 -0700 Subject: [PATCH 4/9] minor: restore some code comment --- programs/fileio.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/programs/fileio.c b/programs/fileio.c index 644a86ef4..6514bf32c 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -537,6 +537,10 @@ static int FIO_removeFile(const char* path) return remove(path); } +/** FIO_openSrcFile() : + * condition : `srcFileName` must be non-NULL. + * optional: `prefs` may be NULL. + * @result : FILE* to `srcFileName`, or NULL if it fails */ static FILE* FIO_openSrcFile(const FIO_prefs_t* const prefs, const char* srcFileName, stat_t* statbuf) { int allowBlockDevices = prefs != NULL ? prefs->allowBlockDevices : 0; From 0bde39a9b32af17a9c21313b7f8cadaf9ae224cd Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 25 Mar 2025 15:22:55 -0700 Subject: [PATCH 5/9] visual studio fix --- programs/util.c | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/programs/util.c b/programs/util.c index 43f2b9408..94c40cc95 100644 --- a/programs/util.c +++ b/programs/util.c @@ -703,21 +703,34 @@ static size_t UTIL_processLines(char* buffer, size_t bufferSize) } /* Create an array of pointers to the lines in a buffer */ -static const char** -UTIL_createLinePointers(char* buffer, size_t numLines, size_t bufferSize) +static const char** UTIL_createLinePointers(char* buffer, size_t numLines, size_t bufferSize) { size_t lineIndex = 0; size_t pos = 0; - const char** linePointers = (const char**)malloc(numLines * sizeof(*linePointers)); - if (linePointers == NULL) return NULL; + void* const bufferPtrs = malloc(numLines * sizeof(const char**)); + const char** const linePointers = (const char**)bufferPtrs; + if (bufferPtrs == NULL) return NULL; while (lineIndex < numLines && pos < bufferSize) { - linePointers[lineIndex++] = buffer + pos; - pos += strlen(buffer + pos) + 1; /* +1 for the finishing `\0` */ + size_t len = 0; + linePointers[lineIndex++] = buffer+pos; + + /* Find the next null terminator, being careful not to go past the buffer */ + while ((pos + len < bufferSize) && buffer[pos + len] != '\0') { + len++; + } + + /* Move past this string and its null terminator */ + pos += len; + if (pos < bufferSize) pos++; /* Skip the null terminator if we're not at buffer end */ } - assert(pos <= bufferSize); - assert(lineIndex == numLines); + /* Verify we processed the expected number of lines */ + if (lineIndex != numLines) { + /* Something went wrong - we didn't find as many lines as expected */ + free(bufferPtrs); + return NULL; + } return linePointers; } From a293cdcb8581b3ff7c460c48fe7fe1b061bdd2e2 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 25 Mar 2025 16:39:42 -0700 Subject: [PATCH 6/9] added CI test --- .github/workflows/dev-long-tests.yml | 5 +- tests/test_process_substitution.bash | 92 ++++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 1 deletion(-) create mode 100755 tests/test_process_substitution.bash diff --git a/.github/workflows/dev-long-tests.yml b/.github/workflows/dev-long-tests.yml index 275b22297..bf287857f 100644 --- a/.github/workflows/dev-long-tests.yml +++ b/.github/workflows/dev-long-tests.yml @@ -28,7 +28,10 @@ jobs: steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # tag=v4.2.2 - name: make test - run: make test + run: | + make test + make -j zstd + ./tests/test_process_substitution.bash ./zstd # lasts ~26mn make-test-macos: diff --git a/tests/test_process_substitution.bash b/tests/test_process_substitution.bash new file mode 100755 index 000000000..586c42838 --- /dev/null +++ b/tests/test_process_substitution.bash @@ -0,0 +1,92 @@ +#!/usr/bin/env bash +# test_process_substitution.bash +# Test zstd's support for process substitution with --filelist + +# Process arguments +ZSTD_PATH="zstd" # Default to using zstd from PATH +if [ $# -ge 1 ]; then + ZSTD_PATH="$1" +fi + +echo "Using zstd executable: $ZSTD_PATH" + +set -e # Exit on error + +# Set up test directory and files +echo "Setting up test environment..." +TEST_DIR="tmp_process_substit" +rm -rf "$TEST_DIR" +mkdir -p "$TEST_DIR" +echo "Content of file 1" > "$TEST_DIR/file1.txt" +echo "Content of file 2" > "$TEST_DIR/file2.txt" +echo "Content of file 3" > "$TEST_DIR/file3.txt" + +# Clean up any previous test artifacts +rm -f "$TEST_DIR/output.zst" "$TEST_DIR/output_echo.zst" "$TEST_DIR/output_cat.zst" +rm -rf "$TEST_DIR/extracted" +mkdir -p "$TEST_DIR/extracted" + +echo "=== Testing process substitution with --filelist ===" + +# Test 1: Basic process substitution with find +echo "Test 1: Basic process substitution (find command)" +"$ZSTD_PATH" --filelist=<(find "$TEST_DIR" -name "*.txt" | sort) -c > "$TEST_DIR/output.zst" + +if [ -f "$TEST_DIR/output.zst" ]; then + echo "✓ Test 1 PASSED: Output file was created" +else + echo "✗ Test 1 FAILED: Output file was not created" + exit 1 +fi + +# Test 2: Process substitution with echo +echo "Test 2: Process substitution (echo command)" +"$ZSTD_PATH" --filelist=<(echo -e "$TEST_DIR/file1.txt\n$TEST_DIR/file2.txt") -c > "$TEST_DIR/output_echo.zst" + +if [ -f "$TEST_DIR/output_echo.zst" ]; then + echo "✓ Test 2 PASSED: Output file was created" +else + echo "✗ Test 2 FAILED: Output file was not created" + exit 1 +fi + +# Test 3: Process substitution with cat +echo "Test 3: Process substitution (cat command)" +echo -e "$TEST_DIR/file1.txt\n$TEST_DIR/file3.txt" > "$TEST_DIR/filelist.txt" +"$ZSTD_PATH" --filelist=<(cat "$TEST_DIR/filelist.txt") -c > "$TEST_DIR/output_cat.zst" + +if [ -f "$TEST_DIR/output_cat.zst" ]; then + echo "✓ Test 3 PASSED: Output file was created" +else + echo "✗ Test 3 FAILED: Output file was not created" + exit 1 +fi + +# Test 4: Verify contents of archives +echo "Test 4: Verifying archive contents" +"$ZSTD_PATH" -d "$TEST_DIR/output.zst" -o "$TEST_DIR/extracted/combined.out" + +if grep -q "Content of file 1" "$TEST_DIR/extracted/combined.out" && + grep -q "Content of file 2" "$TEST_DIR/extracted/combined.out" && + grep -q "Content of file 3" "$TEST_DIR/extracted/combined.out"; then + echo "✓ Test 4 PASSED: All files were correctly archived and extracted" +else + echo "✗ Test 4 FAILED: Not all expected content was found in the extracted file" + exit 1 +fi + +# Test 5: Edge case with empty list +echo "Test 5: Process substitution with empty input" +"$ZSTD_PATH" --filelist=<(echo "") -c > "$TEST_DIR/output_empty.zst" 2>/dev/null || true + +if [ -f "$TEST_DIR/output_empty.zst" ]; then + echo "✓ Test 5 PASSED: Handled empty input gracefully" +else + echo "✓ Test 5 PASSED: Properly rejected empty input" +fi + +# cleanup +rm -rf "$TEST_DIR" + +echo "All tests completed successfully!" + From 8626da73b6596bdd30c1289525db59f09737226d Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 26 Mar 2025 19:21:48 -0700 Subject: [PATCH 7/9] add error message clarification for inputs as process substitution --- programs/fileio.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/programs/fileio.c b/programs/fileio.c index 6514bf32c..6c6faa485 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -660,6 +660,9 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs, #endif if (f == NULL) { DISPLAYLEVEL(1, "zstd: %s: %s\n", dstFileName, strerror(errno)); + if (UTIL_isFileDescriptorPipe(dstFileName)) { + DISPLAYLEVEL(1, "When using process substitution (<(...)), specify an output destination with -o or -c. \n"); + } } else { /* An increased buffer size can provide a significant performance * boost on some platforms. Note that providing a NULL buf with a From 0bdeb1d20451ae8dddffa19d9ff67917cf962a59 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 26 Mar 2025 21:33:26 -0700 Subject: [PATCH 8/9] fix test --- .../compress-file-to-dir-without-write-perm.sh.stderr.exact | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/cli-tests/file-stat/compress-file-to-dir-without-write-perm.sh.stderr.exact b/tests/cli-tests/file-stat/compress-file-to-dir-without-write-perm.sh.stderr.exact index 95deaf2b1..515b63c60 100644 --- a/tests/cli-tests/file-stat/compress-file-to-dir-without-write-perm.sh.stderr.exact +++ b/tests/cli-tests/file-stat/compress-file-to-dir-without-write-perm.sh.stderr.exact @@ -23,4 +23,6 @@ Trace:FileStat: > UTIL_stat(-1, out/file.zst) Trace:FileStat: < 0 Trace:FileStat: < 0 zstd: out/file.zst: Permission denied +Trace:FileStat: > UTIL_isFileDescriptorPipe(out/file.zst) +Trace:FileStat: < 0 zstd: can't stat out/file.zst : Permission denied -- ignored From 2f9627863fb7ecd93b5f7f62b24ba9c7d7af1f77 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 28 Mar 2025 10:02:37 -0700 Subject: [PATCH 9/9] update error message --- programs/fileio.c | 5 +++-- .../compress-file-to-dir-without-write-perm.sh.stderr.exact | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 6c6faa485..dc7636949 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -659,9 +659,10 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs, } #endif if (f == NULL) { - DISPLAYLEVEL(1, "zstd: %s: %s\n", dstFileName, strerror(errno)); if (UTIL_isFileDescriptorPipe(dstFileName)) { - DISPLAYLEVEL(1, "When using process substitution (<(...)), specify an output destination with -o or -c. \n"); + DISPLAYLEVEL(1, "zstd: error: no output specified (use -o or -c). \n"); + } else { + DISPLAYLEVEL(1, "zstd: %s: %s\n", dstFileName, strerror(errno)); } } else { /* An increased buffer size can provide a significant performance diff --git a/tests/cli-tests/file-stat/compress-file-to-dir-without-write-perm.sh.stderr.exact b/tests/cli-tests/file-stat/compress-file-to-dir-without-write-perm.sh.stderr.exact index 515b63c60..9f254f8c2 100644 --- a/tests/cli-tests/file-stat/compress-file-to-dir-without-write-perm.sh.stderr.exact +++ b/tests/cli-tests/file-stat/compress-file-to-dir-without-write-perm.sh.stderr.exact @@ -22,7 +22,7 @@ Trace:FileStat: > UTIL_isRegularFile(out/file.zst) Trace:FileStat: > UTIL_stat(-1, out/file.zst) Trace:FileStat: < 0 Trace:FileStat: < 0 -zstd: out/file.zst: Permission denied Trace:FileStat: > UTIL_isFileDescriptorPipe(out/file.zst) Trace:FileStat: < 0 +zstd: out/file.zst: Permission denied zstd: can't stat out/file.zst : Permission denied -- ignored