Re: [PATCH 2/2] test: cmd: Add gzwrite chunk boundary regression test

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