Hi Aristo,
On 2026-08-06T10:36:59, Aristo Chen <[email protected]> wrote:
> test: cmd: Add gzwrite chunk boundary regression test
>
> Add a deterministic regression test for the gzwrite() case where a
> decompression input chunk is exhausted at exactly the same time as
> the write buffer fills up. Build a gzip file by hand from two 1 KiB
> stored deflate blocks and pick a chunk size that covers exactly the
> first block header plus its payload, so that with a 1 KiB write
> buffer the first input chunk runs out precisely when the write buffer
> is full.
>
> Unlike the existing random data test, which only hits this corner
> case for rare byte patterns (about 1 percent of runs on sandbox64),
> this test fails 20 out of 20 runs without the preceding gunzip fix:
>
> Error: inflate() returned -5
>
> and passed 100 out of 100 runs with it.
>
> Signed-off-by: Aristo Chen <[email protected]>
>
> test/cmd/unzip.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 101 insertions(+), 2 deletions(-)
> diff --git a/test/cmd/unzip.c b/test/cmd/unzip.c
> @@ -132,3 +140,94 @@ static int dm_test_cmd_zip_gzwrite(struct
> unit_test_state *uts)
> + /* Input chunk: exactly one stored block header plus its payload */
> + env_set_ulong("gzwrite_chunk", 5 + blk);
> + ut_assertok(run_commandf("gzwrite mmc 9 %lx %zx %zx", loadaddr,
> + gzlen, blk));
> + ut_assert_skip_to_line("\t%zu bytes, crc 0x%08x", rawsize, crc);
> + env_set("gzwrite_chunk", NULL);
We should have assrrts on the env_set...() functions too.
If any earlier ut_assert*() fires between the env_set_ulong() and the
env_set(..., NULL), gzwrite_chunk leaks into subsequent tests. Ideally
we would put this code into its own function which does the core part,
then call ut_assertok() on a function with the test code. I'm not sure
how much this matters, though.
> diff --git a/test/cmd/unzip.c b/test/cmd/unzip.c
> @@ -132,3 +140,94 @@ static int dm_test_cmd_zip_gzwrite(struct
> unit_test_state *uts)
> + const size_t blk = SZ_1K;
> + const size_t rawsize = 2 * blk;
> ...
> + u8 raw[2 * SZ_1K];
Please use rawsize (or sizeof(raw)) here rather than repeating 2 *
SZ_1K. Same for the 5 + blk chunk size - a named constant for the
stored-block header would make the intent clearer than the bare 5.
> diff --git a/test/cmd/unzip.c b/test/cmd/unzip.c
> @@ -132,3 +140,94 @@ static int dm_test_cmd_zip_gzwrite(struct
> unit_test_state *uts)
> + ut_assertok(run_commandf("mmc read %lx 0 %zx", decaddr,
> + rawsize / 512));
> + ut_assert_nextline("MMC read: dev # 9, block # 0, count %zu ... %zu
> blocks read: OK",
> + rawsize / 512, rawsize / 512);
Since you already have mmc_dev_desc, using mmc_dev_desc->blksz would
be more robust than the literal 512.
Regards,
Simon