Repository navigation
Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The CLI reads compression levels as 32-bit unsigned values and casts them to
int, so a level between 2^31 and 2^32-1 wraps around to a negative (fast) level instead of being treated as "too high":-99andZSTD_CLEVEL=+99are clamped to the maximum level (there is a test for it), and values above 2^32-1 are rejected, but the range in between silently selected a fast level.Changes, all in
programs/zstdcli.c:-#and the benchmark range-e#go through a newreadCLevelFromChar(), which returnsINT_MAXfor a value aboveINT_MAX; the existing clamp then turns it into the maximum level, like-99.ZSTD_CLEVELclamps the absolute value the same way before applying the sign.readIntFromChar()(used by--adapt=min=,max=) now rejects a value aboveINT_MAXwith its existing "numeric value overflows 32-bit int" error, which it previously printed only aboveUINT_MAX. This also removes the signed overflow of(int)result * signfor-2147483648.Tests:
tests/cli-tests/compression/levels.sh:zstd -3000000000andZSTD_CLEVEL=+3000000000must produce the same output as-19(expected stderr updated). Fails ondev(the outputs differ), passes with the change;compression/levels.shandcompression/basic.shpass withrun.py.tests/playTests.sh:--adapt=max=4294967295must fail (it exited 0 ondev; it now exits 1 with the overflow error). I checked this case by hand against both builds; I did not run the wholeplayTests.sh.Built with gcc on Linux x86-64 (
-DZSTD_MULTITHREAD -DZSTD_DISABLE_ASM).AI disclosure (CONTRIBUTING rule 10): I used an AI assistant to help find this issue and write the change; I reviewed the code and ran the tests above.