Minor fixes and improvements minideflate

[nmoinvaz]

No description provided.

[Dead2]

Still not quite right for some reason.
Now minideflate hangs forever during inflate.
Probably stuck in the loop instead of erroring out like before.

[KungFuJesus]

Heh, good thing CI doesn't invoke that tool.

[codecov]

Codecov Report

Merging #1175 (1d5b786) into develop (321028c) will increase coverage by 5.18%.
The diff coverage is 20.00%.

@@             Coverage Diff             @@
##           develop    #1175      +/-   ##
===========================================
+ Coverage    81.99%   87.17%   +5.18%     
===========================================
  Files           97      112      +15     
  Lines         9080     9646     +566     
  Branches      1440     2444    +1004     
===========================================
+ Hits          7445     8409     +964     
+ Misses        1068      988      -80     
+ Partials       567      249     -318     

Flag	Coverage Δ
macos_clang	27.58% <ø> (-42.21%)	⬇️
macos_gcc	70.78% <3.70%> (+1.82%)	⬆️
ubuntu_clang	85.68% <22.50%> (+9.61%)	⬆️
ubuntu_clang_debug	85.46% <20.00%> (+9.36%)	⬆️
ubuntu_clang_inflate_allow_invalid_dist	84.89% <22.50%> (+8.96%)	⬆️
ubuntu_clang_inflate_strict	85.63% <22.50%> (+11.24%)	⬆️
ubuntu_clang_mmap	85.10% <22.50%> (+9.06%)	⬆️
ubuntu_clang_pigz	39.71% <ø> (+5.71%)	⬆️
ubuntu_clang_pigz_no_optim	39.97% <ø> (-0.07%)	⬇️
ubuntu_clang_pigz_no_threads	39.29% <ø> (+5.64%)	⬆️
ubuntu_clang_reduced_mem	85.87% <22.50%> (+9.67%)	⬆️
ubuntu_gcc	72.81% <0.00%> (-1.70%)	⬇️
ubuntu_gcc_aarch64	72.45% <0.00%> (-3.47%)	⬇️
ubuntu_gcc_aarch64_compat_no_opt	69.84% <0.00%> (-3.76%)	⬇️
ubuntu_gcc_aarch64_no_acle	70.73% <0.00%> (-4.70%)	⬇️
ubuntu_gcc_aarch64_no_neon	70.69% <0.00%> (-4.86%)	⬇️
ubuntu_gcc_armhf	72.44% <0.00%> (-3.48%)	⬇️
ubuntu_gcc_armhf_compat_no_opt	69.75% <0.00%> (-3.93%)	⬇️
ubuntu_gcc_armhf_no_acle	72.46% <0.00%> (-3.72%)	⬇️
ubuntu_gcc_armhf_no_neon	72.40% <0.00%> (-3.91%)	⬇️
ubuntu_gcc_armsf	72.44% <0.00%> (-3.48%)	⬇️
ubuntu_gcc_armsf_compat_no_opt	69.75% <0.00%> (-3.93%)	⬇️
ubuntu_gcc_benchmark	74.33% <0.00%> (-2.94%)	⬇️
ubuntu_gcc_compat_no_opt	71.72% <0.00%> (-2.80%)	⬇️
ubuntu_gcc_compat_sprefix	73.73% <0.00%> (-1.78%)	⬇️
ubuntu_gcc_mingw_i686	0.00% <0.00%> (ø)
ubuntu_gcc_mingw_x86_64	0.00% <0.00%> (ø)
ubuntu_gcc_no_avx2	71.54% <0.00%> (-4.30%)	⬇️
ubuntu_gcc_no_ctz	73.65% <0.00%> (-2.92%)	⬇️
ubuntu_gcc_no_ctzll	73.41% <0.00%> (-2.89%)	⬇️
ubuntu_gcc_no_pclmulqdq	70.45% <0.00%> (-3.98%)	⬇️
ubuntu_gcc_no_sse2	71.32% <0.00%> (-2.94%)	⬇️
ubuntu_gcc_no_sse4	71.26% <0.00%> (-3.80%)	⬇️
ubuntu_gcc_o3	73.76% <0.00%> (+0.36%)	⬆️
ubuntu_gcc_osb	∅ <ø> (∅)
ubuntu_gcc_pigz	37.80% <ø> (+3.41%)	⬆️
ubuntu_gcc_pigz_aarch64	38.27% <ø> (+0.58%)	⬆️
ubuntu_gcc_ppc	73.14% <0.00%> (-1.61%)	⬇️
ubuntu_gcc_ppc64	73.91% <0.00%> (-2.36%)	⬇️
ubuntu_gcc_ppc64le	73.10% <0.00%> (-1.80%)	⬇️
ubuntu_gcc_ppc_no_power8	74.13% <0.00%> (-3.23%)	⬇️
ubuntu_gcc_s390x	74.14% <0.00%> (-3.58%)	⬇️
ubuntu_gcc_s390x_dfltcc	72.86% <0.00%> (+0.69%)	⬆️
ubuntu_gcc_s390x_dfltcc_compat	70.53% <0.00%> (-0.85%)	⬇️
ubuntu_gcc_s390x_no_crc32	73.89% <0.00%> (-3.57%)	⬇️
ubuntu_gcc_sparc64	74.02% <0.00%> (-3.45%)	⬇️
ubuntu_gcc_sprefix	73.14% <0.00%> (-2.14%)	⬇️
win64_gcc	72.72% <3.70%> (∅)
win64_gcc_compat_no_opt	72.42% <3.70%> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
test/minideflate.c	56.14% <20.00%> (-3.21%)	⬇️
test/benchmarks/benchmark_compare256.cc	42.85% <0.00%> (-51.59%)	⬇️
arch/s390/dfltcc_detail.h	54.90% <0.00%> (-45.10%)	⬇️
test/benchmarks/benchmark_adler32.cc	50.00% <0.00%> (-43.34%)	⬇️
arch/x86/chunkset_sse2.c	61.11% <0.00%> (-38.89%)	⬇️
test/benchmarks/benchmark_crc32.cc	57.14% <0.00%> (-36.20%)	⬇️
arch/x86/slide_hash_sse2.c	64.70% <0.00%> (-29.05%)	⬇️
test/benchmarks/benchmark_slidehash.cc	72.00% <0.00%> (-23.46%)	⬇️
chunkset.c	94.11% <0.00%> (-5.89%)	⬇️
compare256.c	84.33% <0.00%> (-3.01%)	⬇️
... and 107 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 321028c...1d5b786. Read the comment docs.

[Dead2]

Uhm, it seems this is wrong actually.

According to documentation, windowbits should be:
-1 to -15 is raw deflate without header or trailer
0 to 15 is deflate wrapper (adler32)
16 to 31 is gzip wrapper (crc32c)
32 to 47 is wrapper auto-detect (but not raw deflate)

So, doesn't that mean that we were already doing deflate with adler32?
Also, perhaps minideflate should do decompress with 47 by default? (would need to be tested first of course)

[nmoinvaz]

@Dead2 it looks like you are correct about the window_bits. I've updated the help usage to reflect it. And minideflate does use adler32 by default all your benchmarks before were valid. And for inflateInit by default it will now auto-detect the wrapper. That may have been the issue with the Z_BUF_ERROR that you were experiencing.

If you still get the Z_BUF_ERROR can you tell me which file you are testing against? I tried against silesia-small.tar and didn't get that error.

[Dead2]

Ok, I still get the error, and I guess I should have looked a bit closer at what is happening, because it is obvious it will error out when told to decompress an empty file.. (Maybe we should handle that somehow?)
The problem was that I had -c after the filename, so it silently ignored it and compressed into testfil.gz instead of my redirect into testfil.z, and then that file ended up empty. So this was a clear PEBKAC, sorry about that.

However, this also made me discover a little problem with file names.
When compressing, minideflate will create a file that ends with .gz. However, this is not always the correct filetype, especially not if you use the defaults.
So, I suggest setting the file extension to .z for deflate streams, and .zraw for the raw deflate stream.

Another problem is that it also adds the extension when decompressing. So testfil.gz gets decompressed to testfil.gz.gz, and that easily becomes a mess 😄

An alternative is to also require an output filename as the last parameter, then the user (or our tests) will have to select the name instead. This would probably save a lot of logic to handle the output filename, but probably still smart to just check that input and output are not the same file.

I would like to see a -k keep parameter btw, so it does not delete the source file, as that would save us having to use -c stdout redirection to file in the first place 😅

[mtl1979]

I see no reason why we shouldn't have -k parameter...

[nmoinvaz]

Yes I noticed there was no -k argument also, but that is because it doesn't automatically delete the input file.

[Dead2]

I guess that should perhaps be made clear somehow then, because that differs from how minigzip and many other compression utils works :)

[Dead2]

@nmoinvaz Did you do testing of the new features? If so, then LGTM.

[nmoinvaz]

I did test it while developing. I also tested that each extension was appended properly during compression and removed during decompression. And also tested the auto-wrapper stuff for inflate.

[Dead2]

LGTM