Addressing comments: more comments, cleanup, remove extra function, checksum logic

lib/decompress/zstd_decompress.c

@@ -677,10 +677,10 @@ static size_t ZSTD_decompressFrame(ZSTD_DCtx* dctx,
                         corruption_detected, "");
     }
     if (dctx->fParams.checksumFlag) { /* Frame content checksum verification */
+        RETURN_ERROR_IF(remainingSrcSize<4, checksum_wrong, "");
         if (!dctx->forceIgnoreChecksum) {
             U32 const checkCalc = (U32)XXH64_digest(&dctx->xxhState);
             U32 checkRead;
-            RETURN_ERROR_IF(remainingSrcSize<4, checksum_wrong, "");
             checkRead = MEM_readLE32(ip);
             RETURN_ERROR_IF(checkRead != checkCalc, checksum_wrong, "");
         }
@@ -1403,11 +1403,6 @@ size_t ZSTD_DCtx_setFormat(ZSTD_DCtx* dctx, ZSTD_format_e format)
     return ZSTD_DCtx_setParameter(dctx, ZSTD_d_format, format);
 }
 
-size_t ZSTD_DCtx_setForceIgnoreChecksum(ZSTD_DCtx* dctx, ZSTD_forceIgnoreChecksum_e value)
-{
-    return ZSTD_DCtx_setParameter(dctx, ZSTD_d_forceIgnoreChecksum, value);
-}
-
 ZSTD_bounds ZSTD_dParam_getBounds(ZSTD_dParameter dParam)
 {
     ZSTD_bounds bounds = { 0, 0, 0 };

lib/zstd.h

@@ -1163,6 +1163,7 @@ typedef enum {
 } ZSTD_format_e;
 
 typedef enum {
+    /* Note: this enum controls ZSTD_d_forceIgnoreChecksum */
     ZSTD_d_validateChecksum = 0,
     ZSTD_d_ignoreChecksum = 1
 } ZSTD_forceIgnoreChecksum_e;
@@ -1704,6 +1705,7 @@ ZSTDLIB_API size_t ZSTD_DCtx_setMaxWindowSize(ZSTD_DCtx* dctx, size_t maxWindowS
  * Tells the decompressor to skip checksum validation during decompression, regardless
  * of whether checksumming was specified during compression. This offers some
  * slight performance benefits, and may be useful for debugging.
+ * Param has values of type ZSTD_forceIgnoreChecksum_e
  */
 #define ZSTD_d_forceIgnoreChecksum ZSTD_d_experimentalParam3
 
@@ -1714,11 +1716,6 @@ ZSTDLIB_API size_t ZSTD_DCtx_setMaxWindowSize(ZSTD_DCtx* dctx, size_t maxWindowS
  * @return : 0, or an error code (which can be tested using ZSTD_isError()). */
 ZSTDLIB_API size_t ZSTD_DCtx_setFormat(ZSTD_DCtx* dctx, ZSTD_format_e format);
 
-/*! ZSTD_DCtx_setForceIgnoreChecksum() :
- *  Instruct the decoder context to ignore checksums in compressed frame.
- * @return : 0, or an error code (which can be tested using ZSTD_isError()). */
-ZSTDLIB_API size_t ZSTD_DCtx_setForceIgnoreChecksum(ZSTD_DCtx* dctx, ZSTD_forceIgnoreChecksum_e value);
-
 /*! ZSTD_decompressStream_simpleArgs() :
  *  Same as ZSTD_decompressStream(),
  *  but using only integral types as arguments.

programs/fileio.c

@@ -1754,8 +1754,12 @@ static dRess_t FIO_createDResources(FIO_prefs_t* const prefs, const char* dictFi
     if (ress.dctx==NULL)
         EXM_THROW(60, "Error: %s : can't create ZSTD_DStream", strerror(errno));
     CHECK( ZSTD_DCtx_setMaxWindowSize(ress.dctx, prefs->memLimit) );
-    if (!prefs->checksumFlag)
+    if (!prefs->checksumFlag) {
-        CHECK( ZSTD_DCtx_setForceIgnoreChecksum(ress.dctx, ZSTD_d_ignoreChecksum));
+        CHECK( ZSTD_DCtx_setParameter(ress.dctx, ZSTD_d_forceIgnoreChecksum, ZSTD_d_ignoreChecksum));
+    } else {
+        CHECK( ZSTD_DCtx_setParameter(ress.dctx, ZSTD_d_forceIgnoreChecksum, ZSTD_d_validateChecksum));
+    }
+    
     ress.srcBufferSize = ZSTD_DStreamInSize();
     ress.srcBuffer = malloc(ress.srcBufferSize);
     ress.dstBufferSize = ZSTD_DStreamOutSize();

programs/zstdcli.c

@@ -154,6 +154,20 @@ static void usage_advanced(const char* programName)
     DISPLAYOUT( "--output-dir-mirror DIR : processed files are stored into DIR respecting original directory structure \n");
 #endif
 
+
+#ifndef ZSTD_NOCOMPRESS
+    DISPLAYOUT( "--[no-]check : during compression, add XXH64 integrity checksum to frame (default: enabled)");
+#ifndef ZSTD_NODECOMPRESS
+    DISPLAYOUT( ". If specified with -d, decompressor will ignore/validate checksums in compressed frame (default: validate).");
+#endif
+#endif /* ZSTD_NOCOMPRESS */
+#ifndef ZSTD_NODECOMPRESS
+#ifdef ZSTD_NOCOMPRESS
+    DISPLAYOUT( "--[no-]check : during decompression, ignore/validate checksums in compressed frame (default: validate).");
+#endif
+#endif /* ZSTD_NODECOMPRESS */
+    DISPLAYOUT( "\n");
+
     DISPLAYOUT( "--      : All arguments after \"--\" are treated as files \n");
 
 #ifndef ZSTD_NOCOMPRESS
@@ -174,7 +188,6 @@ static void usage_advanced(const char* programName)
     DISPLAYOUT( "--size-hint=# optimize compression parameters for streaming input of approximately this size \n");
     DISPLAYOUT( "--target-compressed-block-size=# : generate compressed block of approximately targeted size \n");
     DISPLAYOUT( "--no-dictID : don't write dictID into header (dictionary compression only) \n");
-    DISPLAYOUT( "--[no-]check : add XXH64 integrity checksum to frame (default: enabled) \n");
     DISPLAYOUT( "--[no-]compress-literals : force (un)compressed literals \n");
 
     DISPLAYOUT( "--format=zstd : compress files to the .zst format (default) \n");
@@ -196,7 +209,6 @@ static void usage_advanced(const char* programName)
     DISPLAYOUT( " -l     : print information about zstd compressed files \n");
     DISPLAYOUT( "--test  : test compressed file integrity \n");
     DISPLAYOUT( " -M#    : Set a memory usage limit for decompression \n");
-    DISPLAYOUT( "--no-check : disable validation of checksums in compressed frame \n");
 # if ZSTD_SPARSE_DEFAULT
     DISPLAYOUT( "--[no-]sparse : sparse mode (default: enabled on file, disabled on stdout) \n");
 # else

tests/fuzzer.c

@@ -547,6 +547,11 @@ static int basicUnitTests(U32 const seed, double compressibility)
     DISPLAYLEVEL(3, "test%3i : decompress with corrupted checksum : ", testNb++);
     {   /* create compressed buffer with checksumming enabled */
         ZSTD_CCtx* const cctx = ZSTD_createCCtx();
+        if (!cctx) {
+            DISPLAY("Not enough memory, aborting\n");
+            testResult = 1;
+            goto _end;
+        }
         CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_checksumFlag, 1) );
         CHECK_VAR(cSize, ZSTD_compress2(cctx,
                             compressedBuffer, compressedBufferSize,
@@ -555,26 +560,25 @@ static int basicUnitTests(U32 const seed, double compressibility)
     }
     {   /* copy the compressed buffer and corrupt the checksum */
         size_t r;
-        char* const corruptedChecksumCompressedBuffer = (char*)malloc(cSize);
         ZSTD_DCtx* const dctx = ZSTD_createDCtx();
-        if (!corruptedChecksumCompressedBuffer || !dctx) {
+        if (!dctx) {
             DISPLAY("Not enough memory, aborting\n");
             testResult = 1;
             goto _end;
         }
 
-        memcpy(corruptedChecksumCompressedBuffer, compressedBuffer, cSize);
+        ((char*)compressedBuffer)[cSize-1] += 1;
-        corruptedChecksumCompressedBuffer[cSize-1] += 1;
+        r = ZSTD_decompress(decodedBuffer, CNBuffSize, compressedBuffer, cSize);
-        r = ZSTD_decompress(decodedBuffer, CNBuffSize, corruptedChecksumCompressedBuffer, cSize);
         if (!ZSTD_isError(r)) goto _output_error;
         if (ZSTD_getErrorCode(r) != ZSTD_error_checksum_wrong) goto _output_error;
 
         CHECK_Z(ZSTD_DCtx_setForceIgnoreChecksum(dctx, ZSTD_d_ignoreChecksum));
-        r = ZSTD_decompressDCtx(dctx, decodedBuffer, CNBuffSize, corruptedChecksumCompressedBuffer, cSize);
+        r = ZSTD_decompressDCtx(dctx, decodedBuffer, CNBuffSize, compressedBuffer, cSize-1);
+        if (!ZSTD_isError(r)) goto _output_error;   /* wrong checksum size should still throw error */
+        r = ZSTD_decompressDCtx(dctx, decodedBuffer, CNBuffSize, compressedBuffer, cSize);
         if (ZSTD_isError(r)) goto _output_error;
 
         ZSTD_freeDCtx(dctx);
-        free(corruptedChecksumCompressedBuffer);
     }
     DISPLAYLEVEL(3, "OK \n");
 

tests/playTests.sh

@@ -261,8 +261,9 @@ zstd tmp -c --compress-literals    -19      | zstd -t
 zstd -b --fast=1 -i0e1 tmp --compress-literals
 zstd -b --fast=1 -i0e1 tmp --no-compress-literals
 println "test: --no-check for decompression"
-zstd -f tmp --check
+zstd -f tmp -o tmp.zst --check
 zstd -f tmp -o tmp1.zst --no-check
+printf '\xDE\xAD\xBE\xEF' | dd of=tmp.zst bs=1 seek=$(($(wc -c <"tmp.zst") - 4)) count=4 conv=notrunc # corrupt checksum in tmp
 zstd -d -f tmp.zst --no-check
 zstd -d -f tmp1.zst --no-check