Repository navigation
cli: fix --memlimit-decompress= argument parsing - #4916
Open
ZainnQureshii wants to merge 1 commit into
Open
ZainnQureshii wants to merge 1 commit into
ZainnQureshii wants to merge 1 commit into
Conversation
longCommandWArg() matches by prefix, and the --memlimit branch came before --memlimit-decompress, so --memlimit-decompress=8M matched --memlimit and left "-decompress=8M" for NEXT_UINT32, which then read the next argument as the value. Check the longer alias first, and add a CLI test for all three spellings in both argument forms.
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.
What and why
At
programs/zstdcli.c:1105,--memlimitmatched before--memlimit-decompress. Forzstd -dc --memlimit-decompress=8M input.zst, that left-decompress=8MforNEXT_FIELD(programs/zstdcli.c:827), which readinput.zstas the numeric value and exited 1. Moving--memlimit-decompressahead of--memlimitleaves=8M, reads8M, and decompresses successfully with exit 0.tests/cli-tests/basic/memlimit_alias.sh:6checks--memlimit,--memoryand--memlimit-decompress, using both=8Mand separate8Marguments, comparing decompressed output, and checking the window-size diagnostic with=1K.Commands run from the repository root:
make -j4 zstd-releasemake -C tests datagenpython3 tests/cli-tests/run.py basic/memlimit_alias.shpython3 tests/cli-tests/run.py tests/cli-tests/basic/*.shmake checktests/playTests.sh:521: writing to/dev/zerofails withOperation not permitted. No test totals reported.make staticAnalyzescan-buildis unavailable; analysis did not run.xcrun clang-format -lines=1105:1107 programs/zstdcli.csh -n tests/cli-tests/basic/memlimit_alias.shgit diff --checkChecklist
Duplicate check:
gh pr list -R meta/zstd --state open --search "zstdcli.c" --json number,title,url --limit 100, returned five PRs; their diffs do not change the alias ordering.Not applicable: CLI argument parsing fix.
Tools used
Claude Code (with Codex) found the problem, wrote the patch and regression test, independently attacked the fix, ran verification and prepared this draft. GitHub Copilot then reviewed the diff and found no issues.
Prepared with AI assistance (Claude Code) and reviewed with GitHub Copilot.
🤖 Generated with Claude Code