From 9b45db7fa6f2491aaecaa410031b4d6bced870f7 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 27 Sep 2018 16:49:08 -0700 Subject: [PATCH 01/10] minor refactoring of --list trying to reduce recurrent patterns. --- programs/fileio.c | 196 ++++++++++++++++++++------------------------- tests/playTests.sh | 9 ++- 2 files changed, 97 insertions(+), 108 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 53f72aa72..c57792aa4 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -1988,22 +1988,19 @@ typedef struct { U32 nbFiles; } fileInfo_t; -/** getFileInfo() : - * Reads information from file, stores in *info - * @return : 0 if successful - * 1 for frame analysis error - * 2 for file not compressed with zstd - * 3 for cases in which file could not be opened. - */ -static int getFileInfo_fileConfirmed(fileInfo_t* info, const char* inFileName){ - int detectError = 0; - FILE* const srcFile = FIO_openSrcFile(inFileName); - if (srcFile == NULL) { - DISPLAY("Error: could not open source file %s\n", inFileName); - return 3; - } - info->compressedSize = UTIL_getFileSize(inFileName); +typedef enum { info_success=0, info_frame_error=1, info_not_zstd=2, info_file_error=3 } InfoError; +#define EXIT_IF(c,n,...) { \ + if (c) { \ + DISPLAYLEVEL(1, __VA_ARGS__); \ + DISPLAYLEVEL(1, " \n"); \ + return n; \ + } \ +} + +static InfoError +FIO_analyzeFrames(fileInfo_t* info, FILE* const srcFile) +{ /* begin analyzing frame */ for ( ; ; ) { BYTE headerBuffer[ZSTD_FRAMEHEADERSIZE_MAX]; @@ -2013,130 +2010,111 @@ static int getFileInfo_fileConfirmed(fileInfo_t* info, const char* inFileName){ && (numBytesRead == 0) && (info->compressedSize > 0) && (info->compressedSize != UTIL_FILESIZE_UNKNOWN) ) { - break; - } - else if (feof(srcFile)) { - DISPLAY("Error: reached end of file with incomplete frame\n"); - detectError = 2; - break; - } - else { - DISPLAY("Error: did not reach end of file but ran out of frames\n"); - detectError = 1; - break; + return 0; /* successful end of file */ } + EXIT_IF(feof(srcFile), info_not_zstd, "Error: reached end of file with incomplete frame"); + EXIT_IF(1, info_frame_error, "Error: did not reach end of file but ran out of frames"); } { U32 const magicNumber = MEM_readLE32(headerBuffer); /* Zstandard frame */ if (magicNumber == ZSTD_MAGICNUMBER) { ZSTD_frameHeader header; U64 const frameContentSize = ZSTD_getFrameContentSize(headerBuffer, numBytesRead); - if (frameContentSize == ZSTD_CONTENTSIZE_ERROR || frameContentSize == ZSTD_CONTENTSIZE_UNKNOWN) { + if ( frameContentSize == ZSTD_CONTENTSIZE_ERROR + || frameContentSize == ZSTD_CONTENTSIZE_UNKNOWN ) { info->decompUnavailable = 1; } else { info->decompressedSize += frameContentSize; } - if (ZSTD_getFrameHeader(&header, headerBuffer, numBytesRead) != 0) { - DISPLAY("Error: could not decode frame header\n"); - detectError = 1; - break; - } + EXIT_IF(ZSTD_getFrameHeader(&header, headerBuffer, numBytesRead) != 0, + info_frame_error, "Error: could not decode frame header"); info->windowSize = header.windowSize; /* move to the end of the frame header */ { size_t const headerSize = ZSTD_frameHeaderSize(headerBuffer, numBytesRead); - if (ZSTD_isError(headerSize)) { - DISPLAY("Error: could not determine frame header size\n"); - detectError = 1; - break; - } - { int const ret = fseek(srcFile, ((long)headerSize)-((long)numBytesRead), SEEK_CUR); - if (ret != 0) { - DISPLAY("Error: could not move to end of frame header\n"); - detectError = 1; - break; - } } } + EXIT_IF(ZSTD_isError(headerSize), 1, "Error: could not determine frame header size"); + EXIT_IF(fseek(srcFile, ((long)headerSize)-((long)numBytesRead), SEEK_CUR) != 0, + info_frame_error, "Error: could not move to end of frame header"); + } - /* skip the rest of the blocks in the frame */ + /* skip all blocks in the frame */ { int lastBlock = 0; do { BYTE blockHeaderBuffer[3]; - size_t const readBytes = fread(blockHeaderBuffer, 1, 3, srcFile); - if (readBytes != 3) { - DISPLAY("There was a problem reading the block header\n"); - detectError = 1; - break; - } + EXIT_IF(fread(blockHeaderBuffer, 1, 3, srcFile) != 3, + info_frame_error, "Error while reading block header"); { U32 const blockHeader = MEM_readLE24(blockHeaderBuffer); U32 const blockTypeID = (blockHeader >> 1) & 3; U32 const isRLE = (blockTypeID == 1); U32 const isWrongBlock = (blockTypeID == 3); long const blockSize = isRLE ? 1 : (long)(blockHeader >> 3); - if (isWrongBlock) { - DISPLAY("Error: unsupported block type \n"); - detectError = 1; - break; - } + EXIT_IF(isWrongBlock, info_frame_error, "Error: unsupported block type"); lastBlock = blockHeader & 1; - { int const ret = fseek(srcFile, blockSize, SEEK_CUR); - if (ret != 0) { - DISPLAY("Error: could not skip to end of block\n"); - detectError = 1; - break; - } } } + EXIT_IF(fseek(srcFile, blockSize, SEEK_CUR) != 0, + info_frame_error, "Error: could not skip to end of block"); + } } while (lastBlock != 1); - - if (detectError) break; } /* check if checksum is used */ { BYTE const frameHeaderDescriptor = headerBuffer[4]; int const contentChecksumFlag = (frameHeaderDescriptor & (1 << 2)) >> 2; if (contentChecksumFlag) { - int const ret = fseek(srcFile, 4, SEEK_CUR); info->usesCheck = 1; - if (ret != 0) { - DISPLAY("Error: could not skip past checksum\n"); - detectError = 1; - break; - } } } + EXIT_IF(fseek(srcFile, 4, SEEK_CUR) != 0, + info_frame_error, "Error: could not skip past checksum"); + } } info->numActualFrames++; } /* Skippable frame */ else if ((magicNumber & 0xFFFFFFF0U) == ZSTD_MAGIC_SKIPPABLE_START) { U32 const frameSize = MEM_readLE32(headerBuffer + 4); long const seek = (long)(8 + frameSize - numBytesRead); - int const ret = LONG_SEEK(srcFile, seek, SEEK_CUR); - if (ret != 0) { - DISPLAY("Error: could not find end of skippable frame\n"); - detectError = 1; - break; - } + EXIT_IF(LONG_SEEK(srcFile, seek, SEEK_CUR) != 0, + info_frame_error, "Error: could not find end of skippable frame"); info->numSkippableFrames++; } /* unknown content */ else { - detectError = 2; - break; + return info_not_zstd; } - } - } /* end analyzing frame */ - fclose(srcFile); - info->nbFiles = 1; - return detectError; + } /* magic number analysis */ + } /* end analyzing frames */ + return info_success; } -static int getFileInfo(fileInfo_t* info, const char* srcFileName) + +static InfoError +getFileInfo_fileConfirmed(fileInfo_t* info, const char* inFileName) { - int const isAFile = UTIL_isRegularFile(srcFileName); - if (!isAFile) { - DISPLAY("Error : %s is not a file", srcFileName); - return 3; - } + InfoError status; + FILE* const srcFile = FIO_openSrcFile(inFileName); + EXIT_IF(srcFile == NULL, info_file_error, "Error: could not open source file %s", inFileName); + + info->compressedSize = UTIL_getFileSize(inFileName); + status = FIO_analyzeFrames(info, srcFile); + + fclose(srcFile); + info->nbFiles = 1; + return status; +} + + +/** getFileInfo() : + * Reads information from file, stores in *info + * @return : InfoError status + */ +static InfoError +getFileInfo(fileInfo_t* info, const char* srcFileName) +{ + EXIT_IF(!UTIL_isRegularFile(srcFileName), + info_file_error, "Error : %s is not a file", srcFileName); return getFileInfo_fileConfirmed(info, srcFileName); } -static void displayInfo(const char* inFileName, const fileInfo_t* info, int displayLevel){ +static void +displayInfo(const char* inFileName, const fileInfo_t* info, int displayLevel) +{ unsigned const unit = info->compressedSize < (1 MB) ? (1 KB) : (1 MB); const char* const unitStr = info->compressedSize < (1 MB) ? "KB" : "MB"; double const windowSizeUnit = (double)info->windowSize / unit; @@ -2197,43 +2175,45 @@ static fileInfo_t FIO_addFInfo(fileInfo_t fi1, fileInfo_t fi2) static int FIO_listFile(fileInfo_t* total, const char* inFileName, int displayLevel){ fileInfo_t info; memset(&info, 0, sizeof(info)); - { int const error = getFileInfo(&info, inFileName); - if (error == 1) { + { InfoError const error = getFileInfo(&info, inFileName); + if (error == info_frame_error) { /* display error, but provide output */ - DISPLAY("An error occurred while getting file info \n"); + DISPLAYLEVEL(1, "Error while parsing %s \n", inFileName); } - else if (error == 2) { + else if (error == info_not_zstd) { DISPLAYOUT("File %s not compressed by zstd \n", inFileName); if (displayLevel > 2) DISPLAYOUT("\n"); return 1; } - else if (error == 3) { + else if (error == info_file_error) { /* error occurred while opening the file */ if (displayLevel > 2) DISPLAYOUT("\n"); return 1; } displayInfo(inFileName, &info, displayLevel); *total = FIO_addFInfo(*total, info); + assert(error>=0 || error<=1); return error; } } -int FIO_listMultipleFiles(unsigned numFiles, const char** filenameTable, int displayLevel){ - unsigned u; - for (u=0; u 1 && displayLevel <= 2) { /* display total */ unsigned const unit = total.compressedSize < (1 MB) ? (1 KB) : (1 MB); const char* const unitStr = total.compressedSize < (1 MB) ? "KB" : "MB"; diff --git a/tests/playTests.sh b/tests/playTests.sh index c7e066b84..8f16e4a7a 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -180,6 +180,8 @@ chmod 400 tmpro.zst $ZSTD -q tmpro && die "should have refused to overwrite read-only file" $ZSTD -q -f tmpro rm -f tmpro tmpro.zst + + $ECHO "test : file removal" $ZSTD -f --rm tmp test ! -f tmp # tmp should no longer be present @@ -196,9 +198,14 @@ $ECHO a | $ZSTD --rm > $INTOVOID # --rm should remain silent rm tmp $ZSTD -f tmp && die "tmp not present : should have failed" test ! -f tmp.zst # tmp.zst should not be created +$ECHO "test : do not delete destination when source is not present" +touch tmp # create destination file +$ZSTD -d -f tmp.zst && die "attempt to decompress a non existing file" +! test -f tmp # destination file should still be present (test disabled temporarily) +rm tmp* + $ECHO "test : compress multiple files" -rm tmp* $ECHO hello > tmp1 $ECHO world > tmp2 $ZSTD tmp1 tmp2 -o "$INTOVOID" From ef1272737bd2f33044497297715a2bb48d8a1ee5 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 27 Sep 2018 18:29:15 -0700 Subject: [PATCH 02/10] fixed minor Visual conversion warnings --- programs/fileio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index c57792aa4..6c9386c7c 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -2010,7 +2010,7 @@ FIO_analyzeFrames(fileInfo_t* info, FILE* const srcFile) && (numBytesRead == 0) && (info->compressedSize > 0) && (info->compressedSize != UTIL_FILESIZE_UNKNOWN) ) { - return 0; /* successful end of file */ + return info_success; } EXIT_IF(feof(srcFile), info_not_zstd, "Error: reached end of file with incomplete frame"); EXIT_IF(1, info_frame_error, "Error: did not reach end of file but ran out of frames"); @@ -2031,7 +2031,7 @@ FIO_analyzeFrames(fileInfo_t* info, FILE* const srcFile) info->windowSize = header.windowSize; /* move to the end of the frame header */ { size_t const headerSize = ZSTD_frameHeaderSize(headerBuffer, numBytesRead); - EXIT_IF(ZSTD_isError(headerSize), 1, "Error: could not determine frame header size"); + EXIT_IF(ZSTD_isError(headerSize), info_frame_error, "Error: could not determine frame header size"); EXIT_IF(fseek(srcFile, ((long)headerSize)-((long)numBytesRead), SEEK_CUR) != 0, info_frame_error, "Error: could not move to end of frame header"); } From d987ab5983c5bc1660f53d6026a63569675e18e7 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 28 Sep 2018 09:34:16 -0700 Subject: [PATCH 03/10] fixed unreachable section warning on Visual --- programs/fileio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/fileio.c b/programs/fileio.c index e780d481d..2c4e9f6e8 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -2010,7 +2010,7 @@ FIO_analyzeFrames(fileInfo_t* info, FILE* const srcFile) && (numBytesRead == 0) && (info->compressedSize > 0) && (info->compressedSize != UTIL_FILESIZE_UNKNOWN) ) { - return info_success; + break; /* correct end of file => success */ } EXIT_IF(feof(srcFile), info_not_zstd, "Error: reached end of file with incomplete frame"); EXIT_IF(1, info_frame_error, "Error: did not reach end of file but ran out of frames"); From 05c0a072b718d33e451bf84f4b600a9c1e1ba6aa Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 28 Sep 2018 15:57:35 -0700 Subject: [PATCH 04/10] minor improvement in the multi-format suffix selection --- programs/fileio.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 2c4e9f6e8..be0037c24 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -1914,9 +1914,9 @@ int FIO_decompressMultipleFilenames(const char** srcNamesTable, unsigned nbFiles EXM_THROW(72, "Write error : cannot properly close output file"); } else { size_t suffixSize; - size_t dfnSize = FNSPACE; + size_t dfnbCapacity = FNSPACE; unsigned u; - char* dstFileName = (char*)malloc(FNSPACE); + char* dstFileName = (char*)malloc(dfnbCapacity); if (dstFileName==NULL) EXM_THROW(73, "not enough memory for dstFileName"); for (u=0; u Date: Fri, 28 Sep 2018 16:04:00 -0700 Subject: [PATCH 05/10] changed macro name from EXIT_IF() to RETURN_IF() EXIT could be misunderstood as exit(), which terminates program execution. But the macro only leaves the function, not the program. --- programs/fileio.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index be0037c24..12d24603f 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -1997,7 +1997,7 @@ typedef struct { typedef enum { info_success=0, info_frame_error=1, info_not_zstd=2, info_file_error=3 } InfoError; -#define EXIT_IF(c,n,...) { \ +#define ERROR_IF(c,n,...) { \ if (c) { \ DISPLAYLEVEL(1, __VA_ARGS__); \ DISPLAYLEVEL(1, " \n"); \ @@ -2019,8 +2019,8 @@ FIO_analyzeFrames(fileInfo_t* info, FILE* const srcFile) && (info->compressedSize != UTIL_FILESIZE_UNKNOWN) ) { break; /* correct end of file => success */ } - EXIT_IF(feof(srcFile), info_not_zstd, "Error: reached end of file with incomplete frame"); - EXIT_IF(1, info_frame_error, "Error: did not reach end of file but ran out of frames"); + ERROR_IF(feof(srcFile), info_not_zstd, "Error: reached end of file with incomplete frame"); + ERROR_IF(1, info_frame_error, "Error: did not reach end of file but ran out of frames"); } { U32 const magicNumber = MEM_readLE32(headerBuffer); /* Zstandard frame */ @@ -2033,13 +2033,13 @@ FIO_analyzeFrames(fileInfo_t* info, FILE* const srcFile) } else { info->decompressedSize += frameContentSize; } - EXIT_IF(ZSTD_getFrameHeader(&header, headerBuffer, numBytesRead) != 0, + ERROR_IF(ZSTD_getFrameHeader(&header, headerBuffer, numBytesRead) != 0, info_frame_error, "Error: could not decode frame header"); info->windowSize = header.windowSize; /* move to the end of the frame header */ { size_t const headerSize = ZSTD_frameHeaderSize(headerBuffer, numBytesRead); - EXIT_IF(ZSTD_isError(headerSize), info_frame_error, "Error: could not determine frame header size"); - EXIT_IF(fseek(srcFile, ((long)headerSize)-((long)numBytesRead), SEEK_CUR) != 0, + ERROR_IF(ZSTD_isError(headerSize), info_frame_error, "Error: could not determine frame header size"); + ERROR_IF(fseek(srcFile, ((long)headerSize)-((long)numBytesRead), SEEK_CUR) != 0, info_frame_error, "Error: could not move to end of frame header"); } @@ -2047,16 +2047,16 @@ FIO_analyzeFrames(fileInfo_t* info, FILE* const srcFile) { int lastBlock = 0; do { BYTE blockHeaderBuffer[3]; - EXIT_IF(fread(blockHeaderBuffer, 1, 3, srcFile) != 3, + ERROR_IF(fread(blockHeaderBuffer, 1, 3, srcFile) != 3, info_frame_error, "Error while reading block header"); { U32 const blockHeader = MEM_readLE24(blockHeaderBuffer); U32 const blockTypeID = (blockHeader >> 1) & 3; U32 const isRLE = (blockTypeID == 1); U32 const isWrongBlock = (blockTypeID == 3); long const blockSize = isRLE ? 1 : (long)(blockHeader >> 3); - EXIT_IF(isWrongBlock, info_frame_error, "Error: unsupported block type"); + ERROR_IF(isWrongBlock, info_frame_error, "Error: unsupported block type"); lastBlock = blockHeader & 1; - EXIT_IF(fseek(srcFile, blockSize, SEEK_CUR) != 0, + ERROR_IF(fseek(srcFile, blockSize, SEEK_CUR) != 0, info_frame_error, "Error: could not skip to end of block"); } } while (lastBlock != 1); @@ -2067,7 +2067,7 @@ FIO_analyzeFrames(fileInfo_t* info, FILE* const srcFile) int const contentChecksumFlag = (frameHeaderDescriptor & (1 << 2)) >> 2; if (contentChecksumFlag) { info->usesCheck = 1; - EXIT_IF(fseek(srcFile, 4, SEEK_CUR) != 0, + ERROR_IF(fseek(srcFile, 4, SEEK_CUR) != 0, info_frame_error, "Error: could not skip past checksum"); } } info->numActualFrames++; @@ -2076,7 +2076,7 @@ FIO_analyzeFrames(fileInfo_t* info, FILE* const srcFile) else if ((magicNumber & 0xFFFFFFF0U) == ZSTD_MAGIC_SKIPPABLE_START) { U32 const frameSize = MEM_readLE32(headerBuffer + 4); long const seek = (long)(8 + frameSize - numBytesRead); - EXIT_IF(LONG_SEEK(srcFile, seek, SEEK_CUR) != 0, + ERROR_IF(LONG_SEEK(srcFile, seek, SEEK_CUR) != 0, info_frame_error, "Error: could not find end of skippable frame"); info->numSkippableFrames++; } @@ -2095,7 +2095,7 @@ getFileInfo_fileConfirmed(fileInfo_t* info, const char* inFileName) { InfoError status; FILE* const srcFile = FIO_openSrcFile(inFileName); - EXIT_IF(srcFile == NULL, info_file_error, "Error: could not open source file %s", inFileName); + ERROR_IF(srcFile == NULL, info_file_error, "Error: could not open source file %s", inFileName); info->compressedSize = UTIL_getFileSize(inFileName); status = FIO_analyzeFrames(info, srcFile); @@ -2113,7 +2113,7 @@ getFileInfo_fileConfirmed(fileInfo_t* info, const char* inFileName) static InfoError getFileInfo(fileInfo_t* info, const char* srcFileName) { - EXIT_IF(!UTIL_isRegularFile(srcFileName), + ERROR_IF(!UTIL_isRegularFile(srcFileName), info_file_error, "Error : %s is not a file", srcFileName); return getFileInfo_fileConfirmed(info, srcFileName); } @@ -2179,7 +2179,9 @@ static fileInfo_t FIO_addFInfo(fileInfo_t fi1, fileInfo_t fi2) return total; } -static int FIO_listFile(fileInfo_t* total, const char* inFileName, int displayLevel){ +static int +FIO_listFile(fileInfo_t* total, const char* inFileName, int displayLevel) +{ fileInfo_t info; memset(&info, 0, sizeof(info)); { InfoError const error = getFileInfo(&info, inFileName); @@ -2209,7 +2211,7 @@ int FIO_listMultipleFiles(unsigned numFiles, const char** filenameTable, int dis /* ensure no specified input is stdin (needs fseek() capability) */ { unsigned u; for (u=0; u Date: Fri, 28 Sep 2018 18:19:23 -0700 Subject: [PATCH 06/10] regroup name creation logic into its own function for a cleaner main file decompression loop --- programs/fileio.c | 126 ++++++++++++++++++++++++++-------------------- 1 file changed, 71 insertions(+), 55 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 12d24603f..e1903ce5e 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -1895,7 +1895,74 @@ int FIO_decompressFilename(const char* dstFileName, const char* srcFileName, } -#define MAXSUFFIXSIZE 8 +/* FIO_determineDstName() : + * create a destination filename from a srcFileName. + * @return a pointer to it. + * @return == NULL if there is an error */ +static const char* +FIO_determineDstName(const char* srcFileName) +{ + static size_t dfnbCapacity = 0; + static char* dstFileNameBuffer = NULL; /* using static allocation : this function cannot be multi-threaded */ + + size_t const sfnSize = strlen(srcFileName); + size_t suffixSize; + const char* const suffixPtr = strrchr(srcFileName, '.'); + if (suffixPtr == NULL) { + DISPLAYLEVEL(1, "zstd: %s: unknown suffix -- ignored \n", + srcFileName); + return NULL; + } + suffixSize = strlen(suffixPtr); + + /* check suffix is authorized */ + if (sfnSize <= suffixSize + || ( strcmp(suffixPtr, ZSTD_EXTENSION) + #ifdef ZSTD_GZDECOMPRESS + && strcmp(suffixPtr, GZ_EXTENSION) + #endif + #ifdef ZSTD_LZMADECOMPRESS + && strcmp(suffixPtr, XZ_EXTENSION) + && strcmp(suffixPtr, LZMA_EXTENSION) + #endif + #ifdef ZSTD_LZ4DECOMPRESS + && strcmp(suffixPtr, LZ4_EXTENSION) + #endif + ) ) { + const char* suffixlist = ZSTD_EXTENSION + #ifdef ZSTD_GZDECOMPRESS + "/" GZ_EXTENSION + #endif + #ifdef ZSTD_LZMADECOMPRESS + "/" XZ_EXTENSION "/" LZMA_EXTENSION + #endif + #ifdef ZSTD_LZ4DECOMPRESS + "/" LZ4_EXTENSION + #endif + ; + DISPLAYLEVEL(1, "zstd: %s: unknown suffix (%s expected) -- ignored \n", + srcFileName, suffixlist); + return NULL; + } + + /* allocate enough space to write dstFilename into it */ + if (dfnbCapacity+suffixSize <= sfnSize+1) { + free(dstFileNameBuffer); + dfnbCapacity = sfnSize + 20; + dstFileNameBuffer = (char*)malloc(dfnbCapacity); + if (dstFileNameBuffer==NULL) + EXM_THROW(74, "not enough memory for dstFileName"); + } + + /* return dst name == src name truncated from suffix */ + memcpy(dstFileNameBuffer, srcFileName, sfnSize - suffixSize); + dstFileNameBuffer[sfnSize-suffixSize] = '\0'; + return dstFileNameBuffer; + + /* note : dstFileNameBuffer memory is not going to be free */ +} + + int FIO_decompressMultipleFilenames(const char** srcNamesTable, unsigned nbFiles, const char* outFileName, const char* dictFileName) @@ -1913,65 +1980,14 @@ int FIO_decompressMultipleFilenames(const char** srcNamesTable, unsigned nbFiles if (fclose(ress.dstFile)) EXM_THROW(72, "Write error : cannot properly close output file"); } else { - size_t suffixSize; - size_t dfnbCapacity = FNSPACE; unsigned u; - char* dstFileName = (char*)malloc(dfnbCapacity); - if (dstFileName==NULL) - EXM_THROW(73, "not enough memory for dstFileName"); for (u=0; u Date: Mon, 1 Oct 2018 14:04:00 -0700 Subject: [PATCH 07/10] zstd -d -f do no longer erase destination file when source file does not exist (#1082) --- programs/fileio.c | 138 +++++++++++++++++++++++++-------------------- tests/playTests.sh | 2 +- 2 files changed, 77 insertions(+), 63 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index e1903ce5e..a2c4ded1d 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -1792,11 +1792,72 @@ static int FIO_decompressFrames(dRess_t ress, FILE* srcFile, return 0; } +/** FIO_decompressDstFile() : + open `dstFileName`, + or path-through if ress.dstFile is already != 0, + then start decompression process (FIO_decompressFrames()). + @return : 0 : OK + 1 : operation aborted +*/ +static int FIO_decompressDstFile(dRess_t ress, FILE* srcFile, + const char* dstFileName, const char* srcFileName) +{ + int result; + stat_t statbuf; + int transfer_permissions = 0; + int releaseDstFile = 0; + + if (ress.dstFile == NULL) { + releaseDstFile = 1; + + ress.dstFile = FIO_openDstFile(dstFileName); + if (ress.dstFile==0) return 1; + + /* Must only be added after FIO_openDstFile() succeeds. + * Otherwise we may delete the destination file if it already exists, + * and the user presses Ctrl-C when asked if they wish to overwrite. + */ + addHandler(dstFileName); + + if ( strcmp(srcFileName, stdinmark) /* special case : don't transfer permissions from stdin */ + && UTIL_getFileStat(srcFileName, &statbuf) ) + transfer_permissions = 1; + } + + + result = FIO_decompressFrames(ress, srcFile, dstFileName, srcFileName); + + if (releaseDstFile) { + FILE* const dstFile = ress.dstFile; + clearHandler(); + ress.dstFile = NULL; + if (fclose(dstFile)) { + DISPLAYLEVEL(1, "zstd: %s: %s \n", dstFileName, strerror(errno)); + result = 1; + } + + 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 */ + } + signal(SIGINT, SIG_DFL); + } + + return result; +} + /** FIO_decompressSrcFile() : - Decompression `srcFileName` into `ress.dstFile` + Open `srcFileName`, transfer control to decompressDstFile() @return : 0 : OK - 1 : operation not started + 1 : error */ static int FIO_decompressSrcFile(dRess_t ress, const char* dstFileName, const char* srcFileName) { @@ -1812,15 +1873,15 @@ static int FIO_decompressSrcFile(dRess_t ress, const char* dstFileName, const ch if (srcFile==NULL) return 1; ress.srcBufferLoaded = 0; - result = FIO_decompressFrames(ress, srcFile, dstFileName, srcFileName); + result = FIO_decompressDstFile(ress, srcFile, dstFileName, srcFileName); /* Close file */ if (fclose(srcFile)) { DISPLAYLEVEL(1, "zstd: %s: %s \n", srcFileName, strerror(errno)); /* error should not happen */ return 1; } - if ( g_removeSrcFile /* --rm */ - && (result==0) /* decompression successful */ + if ( g_removeSrcFile /* --rm */ + && (result==0) /* decompression successful */ && strcmp(srcFileName, stdinmark) ) /* not stdin */ { /* We must clear the handler, since after this point calling it would * delete both the source and destination files. @@ -1835,60 +1896,13 @@ static int FIO_decompressSrcFile(dRess_t ress, const char* dstFileName, const ch } -/** FIO_decompressFile_extRess() : - decompress `srcFileName` into `dstFileName` - @return : 0 : OK - 1 : operation aborted (src not available, dst already taken, etc.) -*/ -static int FIO_decompressDstFile(dRess_t ress, - const char* dstFileName, const char* srcFileName) -{ - int result; - stat_t statbuf; - int stat_result = 0; - - ress.dstFile = FIO_openDstFile(dstFileName); - if (ress.dstFile==0) return 1; - /* Must ony be added after FIO_openDstFile() succeeds. - * Otherwise we may delete the destination file if at already exists, and - * the user presses Ctrl-C when asked if they wish to overwrite. - */ - addHandler(dstFileName); - - if ( strcmp(srcFileName, stdinmark) - && UTIL_getFileStat(srcFileName, &statbuf) ) - stat_result = 1; - result = FIO_decompressSrcFile(ress, dstFileName, srcFileName); - clearHandler(); - - if (fclose(ress.dstFile)) { - DISPLAYLEVEL(1, "zstd: %s: %s \n", dstFileName, strerror(errno)); - result = 1; - } - - 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 */ - && stat_result ) /* file permissions correctly extracted from src */ - UTIL_setFileStat(dstFileName, &statbuf); /* transfer file permissions from src into dst */ - } - - signal(SIGINT, SIG_DFL); - - return result; -} - int FIO_decompressFilename(const char* dstFileName, const char* srcFileName, const char* dictFileName) { dRess_t const ress = FIO_createDResources(dictFileName); - int const decodingError = FIO_decompressDstFile(ress, dstFileName, srcFileName); + int const decodingError = FIO_decompressSrcFile(ress, dstFileName, srcFileName); FIO_freeDResources(ress); return decodingError; @@ -1963,12 +1977,12 @@ FIO_determineDstName(const char* srcFileName) } -int FIO_decompressMultipleFilenames(const char** srcNamesTable, unsigned nbFiles, - const char* outFileName, - const char* dictFileName) +int +FIO_decompressMultipleFilenames(const char* srcNamesTable[], unsigned nbFiles, + const char* outFileName, + const char* dictFileName) { - int skippedFiles = 0; - int missingFiles = 0; + int error = 0; dRess_t ress = FIO_createDResources(dictFileName); if (outFileName) { @@ -1976,7 +1990,7 @@ int FIO_decompressMultipleFilenames(const char** srcNamesTable, unsigned nbFiles ress.dstFile = FIO_openDstFile(outFileName); if (ress.dstFile == 0) EXM_THROW(71, "cannot open %s", outFileName); for (u=0; u Date: Mon, 1 Oct 2018 17:16:34 -0700 Subject: [PATCH 08/10] ./zstd -f do no longer overwrite destination file if source file does not exist (#1082) --- programs/fileio.c | 204 +++++++++++++++++++++++++++------------------ programs/zstdcli.c | 2 +- tests/playTests.sh | 11 ++- 3 files changed, 134 insertions(+), 83 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index a2c4ded1d..b6acb3e16 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -1066,14 +1066,80 @@ FIO_compressFilename_internal(cRess_t ress, } +/*! FIO_compressFilename_dstFile() : + * open dstFileName, or pass-through if ress.dstFile != NULL, + * then start compression with FIO_compressFilename_internal(). + * Manages source removal (--rm) and file permissions transfer. + * note : ress.srcFile must be != NULL, + * so reach this function through FIO_compressFilename_srcFile(). + * @return : 0 : compression completed correctly, + * 1 : pb + */ +static int FIO_compressFilename_dstFile(cRess_t ress, + const char* dstFileName, + const char* srcFileName, + int compressionLevel) +{ + int closeDstFile = 0; + int result; + stat_t statbuf; + int transfer_permissions = 0; + + assert(ress.srcFile != NULL); + + if (ress.dstFile == NULL) { + closeDstFile = 1; + DISPLAYLEVEL(6, "FIO_compressFilename_dstFile: opening dst: %s", dstFileName); + ress.dstFile = FIO_openDstFile(dstFileName); + if (ress.dstFile==NULL) return 1; /* could not open dstFileName */ + /* Must only be added after FIO_openDstFile() succeeds. + * Otherwise we may delete the destination file if it already exists, + * and the user presses Ctrl-C when asked if they wish to overwrite. + */ + addHandler(dstFileName); + + if ( strcmp (srcFileName, stdinmark) + && UTIL_getFileStat(srcFileName, &statbuf)) + transfer_permissions = 1; + } + + result = FIO_compressFilename_internal(ress, dstFileName, srcFileName, compressionLevel); + + if (closeDstFile) { + FILE* const dstFile = ress.dstFile; + ress.dstFile = NULL; + + clearHandler(); + + if (fclose(dstFile)) { /* error closing dstFile */ + DISPLAYLEVEL(1, "zstd: %s: %s \n", dstFileName, strerror(errno)); + 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) { + UTIL_setFileStat(dstFileName, &statbuf); + } + } + + return result; +} + + /*! FIO_compressFilename_srcFile() : - * note : ress.destFile already opened * @return : 0 : compression completed correctly, * 1 : missing or pb opening srcFileName */ -static int FIO_compressFilename_srcFile(cRess_t ress, - const char* dstFileName, const char* srcFileName, - int compressionLevel) +static int +FIO_compressFilename_srcFile(cRess_t ress, + const char* dstFileName, + const char* srcFileName, + int compressionLevel) { int result; @@ -1084,12 +1150,16 @@ static int FIO_compressFilename_srcFile(cRess_t ress, } ress.srcFile = FIO_openSrcFile(srcFileName); - if (!ress.srcFile) return 1; /* srcFile could not be opened */ + if (ress.srcFile == NULL) return 1; /* srcFile could not be opened */ - result = FIO_compressFilename_internal(ress, dstFileName, srcFileName, compressionLevel); + result = FIO_compressFilename_dstFile(ress, dstFileName, srcFileName, compressionLevel); fclose(ress.srcFile); - if (g_removeSrcFile /* --rm */ && !result && strcmp(srcFileName, stdinmark)) { + ress.srcFile = NULL; + if ( g_removeSrcFile /* --rm */ + && result == 0 /* success */ + && strcmp(srcFileName, stdinmark) /* exception : don't erase stdin */ + ) { /* We must clear the handler, since after this point calling it would * delete both the source and destination files. */ @@ -1101,50 +1171,6 @@ static int FIO_compressFilename_srcFile(cRess_t ress, } -/*! FIO_compressFilename_dstFile() : - * @return : 0 : compression completed correctly, - * 1 : pb - */ -static int FIO_compressFilename_dstFile(cRess_t ress, - const char* dstFileName, - const char* srcFileName, - int compressionLevel) -{ - int result; - stat_t statbuf; - int stat_result = 0; - - DISPLAYLEVEL(6, "FIO_compressFilename_dstFile: opening dst: %s", dstFileName); - ress.dstFile = FIO_openDstFile(dstFileName); - if (ress.dstFile==NULL) return 1; /* could not open dstFileName */ - /* Must ony be added after FIO_openDstFile() succeeds. - * Otherwise we may delete the destination file if at already exists, and - * the user presses Ctrl-C when asked if they wish to overwrite. - */ - addHandler(dstFileName); - - if (strcmp (srcFileName, stdinmark) && UTIL_getFileStat(srcFileName, &statbuf)) - stat_result = 1; - result = FIO_compressFilename_srcFile(ress, dstFileName, srcFileName, compressionLevel); - clearHandler(); - - if (fclose(ress.dstFile)) { /* error closing dstFile */ - DISPLAYLEVEL(1, "zstd: %s: %s \n", dstFileName, strerror(errno)); - 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) - && stat_result) - UTIL_setFileStat(dstFileName, &statbuf); - - return result; -} - - int FIO_compressFilename(const char* dstFileName, const char* srcFileName, const char* dictFileName, int compressionLevel, ZSTD_compressionParameters comprParams) @@ -1154,7 +1180,7 @@ int FIO_compressFilename(const char* dstFileName, const char* srcFileName, U64 const srcSize = (fileSize == UTIL_FILESIZE_UNKNOWN) ? ZSTD_CONTENTSIZE_UNKNOWN : fileSize; cRess_t const ress = FIO_createCResources(dictFileName, compressionLevel, srcSize, comprParams); - int const result = FIO_compressFilename_dstFile(ress, dstFileName, srcFileName, compressionLevel); + int const result = FIO_compressFilename_srcFile(ress, dstFileName, srcFileName, compressionLevel); double const seconds = (double)(clock() - start) / CLOCKS_PER_SEC; DISPLAYLEVEL(4, "Completed in %.2f sec \n", seconds); @@ -1164,57 +1190,75 @@ int FIO_compressFilename(const char* dstFileName, const char* srcFileName, } +/* FIO_determineCompressedName() : + * create a destination filename for compressed srcFileName. + * @return a pointer to it. + * This function never returns an error (it may abort() in case of pb) + */ +static const char* +FIO_determineCompressedName(const char* srcFileName, const char* suffix) +{ + static size_t dfnbCapacity = 0; + static char* dstFileNameBuffer = NULL; /* using static allocation : this function cannot be multi-threaded */ + + size_t const sfnSize = strlen(srcFileName); + size_t const suffixSize = strlen(suffix); + + if (dfnbCapacity <= sfnSize+suffixSize+1) { /* resize name buffer */ + free(dstFileNameBuffer); + dfnbCapacity = sfnSize + suffixSize + 30; + dstFileNameBuffer = (char*)malloc(dfnbCapacity); + if (!dstFileNameBuffer) { + EXM_THROW(30, "zstd: %s", strerror(errno)); + } } + strncpy(dstFileNameBuffer, srcFileName, sfnSize+1 /* Include null */); + strncat(dstFileNameBuffer, suffix, suffixSize); + + return dstFileNameBuffer; +} + + +/* FIO_compressMultipleFilenames() : + * compress nbFiles files + * into one destination (outFileName) + * or into one file each (outFileName == NULL, but suffix != NULL). + */ int FIO_compressMultipleFilenames(const char** inFileNamesTable, unsigned nbFiles, const char* outFileName, const char* suffix, const char* dictFileName, int compressionLevel, ZSTD_compressionParameters comprParams) { - int missed_files = 0; - size_t dfnSize = FNSPACE; - char* dstFileName = (char*)malloc(FNSPACE); - size_t const suffixSize = suffix ? strlen(suffix) : 0; + int error = 0; U64 const firstFileSize = UTIL_getFileSize(inFileNamesTable[0]); U64 const firstSrcSize = (firstFileSize == UTIL_FILESIZE_UNKNOWN) ? ZSTD_CONTENTSIZE_UNKNOWN : firstFileSize; U64 const srcSize = (nbFiles != 1) ? ZSTD_CONTENTSIZE_UNKNOWN : firstSrcSize ; cRess_t ress = FIO_createCResources(dictFileName, compressionLevel, srcSize, comprParams); /* init */ - if (dstFileName==NULL) - EXM_THROW(27, "FIO_compressMultipleFilenames : allocation error for dstFileName"); - if (outFileName == NULL && suffix == NULL) - EXM_THROW(28, "FIO_compressMultipleFilenames : dst unknown"); /* should never happen */ + assert(outFileName != NULL || suffix != NULL); - /* loop on each file */ - if (outFileName != NULL) { - unsigned u; + if (outFileName != NULL) { /* output into a single destination (stdout typically) */ ress.dstFile = FIO_openDstFile(outFileName); - if (ress.dstFile==NULL) { /* could not open outFileName */ - missed_files = nbFiles; + if (ress.dstFile == NULL) { /* could not open outFileName */ + error = 1; } else { + unsigned u; for (u=0; u $INTOVOID # --rm should remain silent rm tmp $ZSTD -f tmp && die "tmp not present : should have failed" test ! -f tmp.zst # tmp.zst should not be created -$ECHO "test : do not delete destination when source is not present" +$ECHO "test : -d -f do not delete destination when source is not present" touch tmp # create destination file $ZSTD -d -f tmp.zst && die "attempt to decompress a non existing file" -test -f tmp # destination file should still be present (test disabled temporarily) +test -f tmp # destination file should still be present +$ECHO "test : -f do not delete destination when source is not present" +rm tmp # erase source file +touch tmp.zst # create destination file +$ZSTD -f tmp && die "attempt to compress a non existing file" +test -f tmp.zst # destination file should still be present rm tmp* @@ -824,6 +829,7 @@ roundTripTest -g1M -P50 "1 --single-thread --long=29" " --zstd=wlog=28 --memory= $ECHO "\n===> adaptive mode " roundTripTest -g270000000 " --adapt" roundTripTest -g27000000 " --adapt=min=1,max=4" +$ECHO "===> test: --adapt must fail on incoherent bounds " ./datagen > tmp $ZSTD -f -vv --adapt=min=10,max=9 tmp && die "--adapt must fail on incoherent bounds" @@ -834,6 +840,7 @@ if [ "$1" != "--test-large-data" ]; then fi +############################################################################# $ECHO "\n===> large files tests " From d98733b37e3117b8be0e11ff697f25ba44d9db6a Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 1 Oct 2018 17:50:16 -0700 Subject: [PATCH 09/10] restored backtrace on failure for Linux and Mac OS-X. Note : the backtraces fires up through a trap before the sanitizer get a chance to report. There are situations where the sanitizer report is actually preferable. It might be good to consider a kind of build macro which can disable backtrace when sanitizer is enabled. --- programs/zstdcli.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/programs/zstdcli.c b/programs/zstdcli.c index 6b0c89d46..1545d1cac 100644 --- a/programs/zstdcli.c +++ b/programs/zstdcli.c @@ -547,7 +547,7 @@ int main(int argCount, const char* argv[]) memset(&compressionParams, 0, sizeof(compressionParams)); /* init crash handler */ - //FIO_addAbortHandler(); + FIO_addAbortHandler(); /* command switches */ for (argNb=1; argNb Date: Tue, 2 Oct 2018 15:59:11 -0700 Subject: [PATCH 10/10] fixed static analyzer warnings note : for some reason, scan-build version on my laptop found problems within fastcover.c that scan-build on travisCI does not flag. They are, as usual, false positive : the analyzer does not understand that a table (`offset`) is correctly filled before usage. --- lib/dictBuilder/fastcover.c | 249 ++++++++++++++++++++---------------- programs/fileio.c | 16 ++- 2 files changed, 147 insertions(+), 118 deletions(-) diff --git a/lib/dictBuilder/fastcover.c b/lib/dictBuilder/fastcover.c index 6ce8c8809..dfee45743 100644 --- a/lib/dictBuilder/fastcover.c +++ b/lib/dictBuilder/fastcover.c @@ -245,39 +245,41 @@ static int FASTCOVER_checkParameters(ZDICT_cover_params_t parameters, /** * Clean up a context initialized with `FASTCOVER_ctx_init()`. */ -static void FASTCOVER_ctx_destroy(FASTCOVER_ctx_t *ctx) { - if (!ctx) { - return; - } +static void +FASTCOVER_ctx_destroy(FASTCOVER_ctx_t* ctx) +{ + if (!ctx) return; - free(ctx->freqs); - ctx->freqs = NULL; + free(ctx->freqs); + ctx->freqs = NULL; - free(ctx->offsets); - ctx->offsets = NULL; + free(ctx->offsets); + ctx->offsets = NULL; } /** * Calculate for frequency of hash value of each dmer in ctx->samples */ -static void FASTCOVER_computeFrequency(U32 *freqs, FASTCOVER_ctx_t *ctx){ - const unsigned f = ctx->f; - const unsigned d = ctx->d; - const unsigned skip = ctx->accelParams.skip; - const unsigned readLength = MAX(d, 8); - size_t start; /* start of current dmer */ - size_t i; - for (i = 0; i < ctx->nbTrainSamples; i++) { - size_t currSampleStart = ctx->offsets[i]; - size_t currSampleEnd = ctx->offsets[i+1]; - start = currSampleStart; - while (start + readLength <= currSampleEnd) { - const size_t dmerIndex = FASTCOVER_hashPtrToIndex(ctx->samples + start, f, d); - freqs[dmerIndex]++; - start = start + skip + 1; +static void +FASTCOVER_computeFrequency(U32* freqs, const FASTCOVER_ctx_t* ctx) +{ + const unsigned f = ctx->f; + const unsigned d = ctx->d; + const unsigned skip = ctx->accelParams.skip; + const unsigned readLength = MAX(d, 8); + size_t i; + assert(ctx->nbTrainSamples >= 5); + assert(ctx->nbTrainSamples <= ctx->nbSamples); + for (i = 0; i < ctx->nbTrainSamples; i++) { + size_t start = ctx->offsets[i]; /* start of current dmer */ + size_t const currSampleEnd = ctx->offsets[i+1]; + while (start + readLength <= currSampleEnd) { + const size_t dmerIndex = FASTCOVER_hashPtrToIndex(ctx->samples + start, f, d); + freqs[dmerIndex]++; + start = start + skip + 1; + } } - } } @@ -288,84 +290,100 @@ static void FASTCOVER_computeFrequency(U32 *freqs, FASTCOVER_ctx_t *ctx){ * Returns 1 on success or zero on error. * The context must be destroyed with `FASTCOVER_ctx_destroy()`. */ -static int FASTCOVER_ctx_init(FASTCOVER_ctx_t *ctx, const void *samplesBuffer, - const size_t *samplesSizes, unsigned nbSamples, - unsigned d, double splitPoint, unsigned f, - FASTCOVER_accel_t accelParams) { - const BYTE *const samples = (const BYTE *)samplesBuffer; - const size_t totalSamplesSize = COVER_sum(samplesSizes, nbSamples); - /* Split samples into testing and training sets */ - const unsigned nbTrainSamples = splitPoint < 1.0 ? (unsigned)((double)nbSamples * splitPoint) : nbSamples; - const unsigned nbTestSamples = splitPoint < 1.0 ? nbSamples - nbTrainSamples : nbSamples; - const size_t trainingSamplesSize = splitPoint < 1.0 ? COVER_sum(samplesSizes, nbTrainSamples) : totalSamplesSize; - const size_t testSamplesSize = splitPoint < 1.0 ? COVER_sum(samplesSizes + nbTrainSamples, nbTestSamples) : totalSamplesSize; - /* Checks */ - if (totalSamplesSize < MAX(d, sizeof(U64)) || - totalSamplesSize >= (size_t)FASTCOVER_MAX_SAMPLES_SIZE) { - DISPLAYLEVEL(1, "Total samples size is too large (%u MB), maximum size is %u MB\n", - (U32)(totalSamplesSize >> 20), (FASTCOVER_MAX_SAMPLES_SIZE >> 20)); - return 0; - } - /* Check if there are at least 5 training samples */ - if (nbTrainSamples < 5) { - DISPLAYLEVEL(1, "Total number of training samples is %u and is invalid\n", nbTrainSamples); - return 0; - } - /* Check if there's testing sample */ - if (nbTestSamples < 1) { - DISPLAYLEVEL(1, "Total number of testing samples is %u and is invalid.\n", nbTestSamples); - return 0; - } - /* Zero the context */ - memset(ctx, 0, sizeof(*ctx)); - DISPLAYLEVEL(2, "Training on %u samples of total size %u\n", nbTrainSamples, - (U32)trainingSamplesSize); - DISPLAYLEVEL(2, "Testing on %u samples of total size %u\n", nbTestSamples, - (U32)testSamplesSize); +static int +FASTCOVER_ctx_init(FASTCOVER_ctx_t* ctx, + const void* samplesBuffer, + const size_t* samplesSizes, unsigned nbSamples, + unsigned d, double splitPoint, unsigned f, + FASTCOVER_accel_t accelParams) +{ + const BYTE* const samples = (const BYTE*)samplesBuffer; + const size_t totalSamplesSize = COVER_sum(samplesSizes, nbSamples); + /* Split samples into testing and training sets */ + const unsigned nbTrainSamples = splitPoint < 1.0 ? (unsigned)((double)nbSamples * splitPoint) : nbSamples; + const unsigned nbTestSamples = splitPoint < 1.0 ? nbSamples - nbTrainSamples : nbSamples; + const size_t trainingSamplesSize = splitPoint < 1.0 ? COVER_sum(samplesSizes, nbTrainSamples) : totalSamplesSize; + const size_t testSamplesSize = splitPoint < 1.0 ? COVER_sum(samplesSizes + nbTrainSamples, nbTestSamples) : totalSamplesSize; - ctx->samples = samples; - ctx->samplesSizes = samplesSizes; - ctx->nbSamples = nbSamples; - ctx->nbTrainSamples = nbTrainSamples; - ctx->nbTestSamples = nbTestSamples; - ctx->nbDmers = trainingSamplesSize - MAX(d, sizeof(U64)) + 1; - ctx->d = d; - ctx->f = f; - ctx->accelParams = accelParams; - - /* The offsets of each file */ - ctx->offsets = (size_t *)malloc((nbSamples + 1) * sizeof(size_t)); - if (!ctx->offsets) { - DISPLAYLEVEL(1, "Failed to allocate scratch buffers\n"); - FASTCOVER_ctx_destroy(ctx); - return 0; - } - - /* Fill offsets from the samplesSizes */ - { - U32 i; - ctx->offsets[0] = 0; - for (i = 1; i <= nbSamples; ++i) { - ctx->offsets[i] = ctx->offsets[i - 1] + samplesSizes[i - 1]; + /* Checks */ + if (totalSamplesSize < MAX(d, sizeof(U64)) || + totalSamplesSize >= (size_t)FASTCOVER_MAX_SAMPLES_SIZE) { + DISPLAYLEVEL(1, "Total samples size is too large (%u MB), maximum size is %u MB\n", + (U32)(totalSamplesSize >> 20), (FASTCOVER_MAX_SAMPLES_SIZE >> 20)); + return 0; } - } - /* Initialize frequency array of size 2^f */ - ctx->freqs = (U32 *)calloc(((U64)1 << f), sizeof(U32)); + /* Check if there are at least 5 training samples */ + if (nbTrainSamples < 5) { + DISPLAYLEVEL(1, "Total number of training samples is %u and is invalid\n", nbTrainSamples); + return 0; + } - DISPLAYLEVEL(2, "Computing frequencies\n"); - FASTCOVER_computeFrequency(ctx->freqs, ctx); + /* Check if there's testing sample */ + if (nbTestSamples < 1) { + DISPLAYLEVEL(1, "Total number of testing samples is %u and is invalid.\n", nbTestSamples); + return 0; + } - return 1; + /* Zero the context */ + memset(ctx, 0, sizeof(*ctx)); + DISPLAYLEVEL(2, "Training on %u samples of total size %u\n", nbTrainSamples, + (U32)trainingSamplesSize); + DISPLAYLEVEL(2, "Testing on %u samples of total size %u\n", nbTestSamples, + (U32)testSamplesSize); + + ctx->samples = samples; + ctx->samplesSizes = samplesSizes; + ctx->nbSamples = nbSamples; + ctx->nbTrainSamples = nbTrainSamples; + ctx->nbTestSamples = nbTestSamples; + ctx->nbDmers = trainingSamplesSize - MAX(d, sizeof(U64)) + 1; + ctx->d = d; + ctx->f = f; + ctx->accelParams = accelParams; + + /* The offsets of each file */ + ctx->offsets = (size_t*)calloc((nbSamples + 1), sizeof(size_t)); + if (ctx->offsets == NULL) { + DISPLAYLEVEL(1, "Failed to allocate scratch buffers \n"); + FASTCOVER_ctx_destroy(ctx); + return 0; + } + + /* Fill offsets from the samplesSizes */ + { U32 i; + ctx->offsets[0] = 0; + assert(nbSamples >= 5); + for (i = 1; i <= nbSamples; ++i) { + ctx->offsets[i] = ctx->offsets[i - 1] + samplesSizes[i - 1]; + } + } + + /* Initialize frequency array of size 2^f */ + ctx->freqs = (U32*)calloc(((U64)1 << f), sizeof(U32)); + if (ctx->freqs == NULL) { + DISPLAYLEVEL(1, "Failed to allocate frequency table \n"); + FASTCOVER_ctx_destroy(ctx); + return 0; + } + + DISPLAYLEVEL(2, "Computing frequencies\n"); + FASTCOVER_computeFrequency(ctx->freqs, ctx); + + return 1; } /** * Given the prepared context build the dictionary. */ -static size_t FASTCOVER_buildDictionary(const FASTCOVER_ctx_t *ctx, U32 *freqs, - void *dictBuffer, size_t dictBufferCapacity, - ZDICT_cover_params_t parameters, U16* segmentFreqs){ +static size_t +FASTCOVER_buildDictionary(const FASTCOVER_ctx_t* ctx, + U32* freqs, + void* dictBuffer, size_t dictBufferCapacity, + ZDICT_cover_params_t parameters, + U16* segmentFreqs) +{ BYTE *const dict = (BYTE *)dictBuffer; size_t tail = dictBufferCapacity; /* Divide the data up into epochs of equal size. @@ -416,10 +434,10 @@ static size_t FASTCOVER_buildDictionary(const FASTCOVER_ctx_t *ctx, U32 *freqs, * Parameters for FASTCOVER_tryParameters(). */ typedef struct FASTCOVER_tryParameters_data_s { - const FASTCOVER_ctx_t *ctx; - COVER_best_t *best; - size_t dictBufferCapacity; - ZDICT_cover_params_t parameters; + const FASTCOVER_ctx_t* ctx; + COVER_best_t* best; + size_t dictBufferCapacity; + ZDICT_cover_params_t parameters; } FASTCOVER_tryParameters_data_t; @@ -428,7 +446,8 @@ typedef struct FASTCOVER_tryParameters_data_s { * This function is thread safe if zstd is compiled with multithreaded support. * It takes its parameters as an *OWNING* opaque pointer to support threading. */ -static void FASTCOVER_tryParameters(void *opaque) { +static void FASTCOVER_tryParameters(void *opaque) +{ /* Save parameters as local variables */ FASTCOVER_tryParameters_data_t *const data = (FASTCOVER_tryParameters_data_t *)opaque; const FASTCOVER_ctx_t *const ctx = data->ctx; @@ -447,8 +466,7 @@ static void FASTCOVER_tryParameters(void *opaque) { /* Copy the frequencies because we need to modify them */ memcpy(freqs, ctx->freqs, ((U64)1 << ctx->f) * sizeof(U32)); /* Build the dictionary */ - { - const size_t tail = FASTCOVER_buildDictionary(ctx, freqs, dict, dictBufferCapacity, + { const size_t tail = FASTCOVER_buildDictionary(ctx, freqs, dict, dictBufferCapacity, parameters, segmentFreqs); const unsigned nbFinalizeSamples = (unsigned)(ctx->nbTrainSamples * ctx->accelParams.finalize / 100); dictBufferCapacity = ZDICT_finalizeDictionary( @@ -474,9 +492,10 @@ _cleanup: } - -static void FASTCOVER_convertToCoverParams(ZDICT_fastCover_params_t fastCoverParams, - ZDICT_cover_params_t *coverParams) { +static void +FASTCOVER_convertToCoverParams(ZDICT_fastCover_params_t fastCoverParams, + ZDICT_cover_params_t* coverParams) +{ coverParams->k = fastCoverParams.k; coverParams->d = fastCoverParams.d; coverParams->steps = fastCoverParams.steps; @@ -486,9 +505,11 @@ static void FASTCOVER_convertToCoverParams(ZDICT_fastCover_params_t fastCoverPar } -static void FASTCOVER_convertToFastCoverParams(ZDICT_cover_params_t coverParams, - ZDICT_fastCover_params_t *fastCoverParams, - unsigned f, unsigned accel) { +static void +FASTCOVER_convertToFastCoverParams(ZDICT_cover_params_t coverParams, + ZDICT_fastCover_params_t* fastCoverParams, + unsigned f, unsigned accel) +{ fastCoverParams->k = coverParams.k; fastCoverParams->d = coverParams.d; fastCoverParams->steps = coverParams.steps; @@ -500,9 +521,12 @@ static void FASTCOVER_convertToFastCoverParams(ZDICT_cover_params_t coverParams, } -ZDICTLIB_API size_t ZDICT_trainFromBuffer_fastCover( - void *dictBuffer, size_t dictBufferCapacity, const void *samplesBuffer, - const size_t *samplesSizes, unsigned nbSamples, ZDICT_fastCover_params_t parameters) { +ZDICTLIB_API size_t +ZDICT_trainFromBuffer_fastCover(void* dictBuffer, size_t dictBufferCapacity, + const void* samplesBuffer, + const size_t* samplesSizes, unsigned nbSamples, + ZDICT_fastCover_params_t parameters) +{ BYTE* const dict = (BYTE*)dictBuffer; FASTCOVER_ctx_t ctx; ZDICT_cover_params_t coverParams; @@ -562,10 +586,13 @@ ZDICTLIB_API size_t ZDICT_trainFromBuffer_fastCover( } -ZDICTLIB_API size_t ZDICT_optimizeTrainFromBuffer_fastCover( - void *dictBuffer, size_t dictBufferCapacity, const void *samplesBuffer, - const size_t *samplesSizes, unsigned nbSamples, - ZDICT_fastCover_params_t *parameters) { +ZDICTLIB_API size_t +ZDICT_optimizeTrainFromBuffer_fastCover( + void* dictBuffer, size_t dictBufferCapacity, + const void* samplesBuffer, + const size_t* samplesSizes, unsigned nbSamples, + ZDICT_fastCover_params_t* parameters) +{ ZDICT_cover_params_t coverParams; FASTCOVER_accel_t accelParams; /* constants */ diff --git a/programs/fileio.c b/programs/fileio.c index b6acb3e16..b66426b1d 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -20,10 +20,12 @@ # define _POSIX_SOURCE 1 /* disable %llu warnings with MinGW on Windows */ #endif -#if defined(__linux__) || (defined(__APPLE__) && defined(__MACH__)) +#if !defined(BACKTRACES_ENABLE) && \ + (defined(__linux__) || (defined(__APPLE__) && defined(__MACH__)) ) # define BACKTRACES_ENABLE 1 #endif + /*-************************************* * Includes ***************************************/ @@ -32,6 +34,7 @@ #include /* fprintf, fopen, fread, _fileno, stdin, stdout */ #include /* malloc, free */ #include /* strcmp, strlen */ +#include #include /* errno */ #include #ifdef BACKTRACES_ENABLE @@ -43,10 +46,8 @@ # include #endif -#include "debug.h" -#include "mem.h" +#include "mem.h" /* U32, U64 */ #include "fileio.h" -#include "util.h" #define ZSTD_STATIC_LINKING_ONLY /* ZSTD_magicNumber, ZSTD_frameHeaderSize_max */ #include "zstd.h" @@ -113,7 +114,7 @@ static UTIL_time_t g_displayClock = UTIL_TIME_INITIALIZER; #define EXM_THROW(error, ...) \ { \ DISPLAYLEVEL(1, "zstd: "); \ - DEBUGLOG(1, "Error defined at %s, line %i : \n", __FILE__, __LINE__); \ + DISPLAYLEVEL(5, "Error defined at %s, line %i : \n", __FILE__, __LINE__); \ DISPLAYLEVEL(1, "error %i : ", error); \ DISPLAYLEVEL(1, __VA_ARGS__); \ DISPLAYLEVEL(1, " \n"); \ @@ -123,7 +124,7 @@ static UTIL_time_t g_displayClock = UTIL_TIME_INITIALIZER; #define CHECK_V(v, f) \ v = f; \ if (ZSTD_isError(v)) { \ - DEBUGLOG(1, "%s \n", #f); \ + DISPLAYLEVEL(5, "%s \n", #f); \ EXM_THROW(11, "%s", ZSTD_getErrorName(v)); \ } #define CHECK(f) { size_t err; CHECK_V(err, f); } @@ -1211,6 +1212,7 @@ FIO_determineCompressedName(const char* srcFileName, const char* suffix) if (!dstFileNameBuffer) { EXM_THROW(30, "zstd: %s", strerror(errno)); } } + assert(dstFileNameBuffer != NULL); strncpy(dstFileNameBuffer, srcFileName, sfnSize+1 /* Include null */); strncat(dstFileNameBuffer, suffix, suffixSize); @@ -1891,7 +1893,6 @@ static int FIO_decompressDstFile(dRess_t ress, FILE* srcFile, && transfer_permissions ) /* file permissions correctly extracted from src */ UTIL_setFileStat(dstFileName, &statbuf); /* transfer file permissions from src into dst */ } - signal(SIGINT, SIG_DFL); } return result; @@ -2013,6 +2014,7 @@ FIO_determineDstName(const char* srcFileName) } /* return dst name == src name truncated from suffix */ + assert(dstFileNameBuffer != NULL); memcpy(dstFileNameBuffer, srcFileName, sfnSize - suffixSize); dstFileNameBuffer[sfnSize-suffixSize] = '\0'; return dstFileNameBuffer;