zstd icon indicating copy to clipboard operation
zstd copied to clipboard

1 GiB file bug

Open ip7z opened this issue 3 years ago • 2 comments

zstd 1.5.2 doesn't reduce the Windows Size from 2 GiB to 1 GiB, if file size is exactly 1 GiB.

To Reproduce

FSUtil File CreateNew 1024 0x40000000
zstd --long=31 -1 --single-thread --no-content-size -f 1024
zstd -l -v 1024.zst

result:

Window Size: 2.00 GiB (2147483648 B)

Expected behavior

Window Size: 1.00 GiB

Source code lines the bug probably in function ZSTD_adjustCParams_internal() in zstd_compress.c:

    const U64 maxWindowResize = 1ULL << (ZSTD_WINDOWLOG_MAX-1);
   ... 
    /* resize windowLog if input is small enough, to use less memory */
    if ( (srcSize < maxWindowResize)

and fixed version probably must use <= instead of <:

    if ( (srcSize <= maxWindowResize)

ip7z avatar Aug 10 '22 06:08 ip7z

Thanks for the report! I will put up a fix before the next release.

embg avatar Aug 16 '22 16:08 embg

I think the original author wanted to be extra safe with the addition that follows. I believe something like this is enough?

hbina@akarin:~/git/zstd$ git diff
diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c
index 59d441b2..44693bf7 100644
--- a/lib/compress/zstd_compress.c
+++ b/lib/compress/zstd_compress.c
@@ -1369,8 +1369,8 @@ ZSTD_adjustCParams_internal(ZSTD_compressionParameters cPar,
     }
 
     /* resize windowLog if input is small enough, to use less memory */
-    if ( (srcSize < maxWindowResize)
-      && (dictSize < maxWindowResize) )  {
+    if ( (srcSize <= maxWindowResize)
+      && (dictSize <= maxWindowResize) )  {
         U32 const tSize = (U32)(srcSize + dictSize);
         static U32 const hashSizeMin = 1 << ZSTD_HASHLOG_MIN;
         U32 const srcLog = (tSize < hashSizeMin) ? ZSTD_HASHLOG_MIN :

hbina avatar Aug 24 '22 09:08 hbina