Re: [PATCH v3 02/11] reftable: fix resource leak in error path
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Dec 13, 2021, 16:19 UTC
- Message-ID
- <211213.86pmq08ogt.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <975a570d388fca79546987f4683fcd33419aad98.1639411309.git.gitgitgadget@gmail.com>
On Mon, Dec 13 2021, Han-Wen Nienhuys via GitGitGadget wrote:
Show 9 quoted lines
> From: Han-Wen Nienhuys <hanwen@google.com>
> [...]
> - return 0;
> +done:
> + if (uncompressed) {
> + reftable_free(uncompressed);
> + }
> + return err;
> }Other things in the codebase don't check for NULL before feeding things to reftable_free(), and our own free() has a coccicheck rule to catch this sort of code, we should probably add reftable_free to that list...
Show 18 quoted lines
>
> static uint32_t block_reader_restart_offset(struct block_reader *br, int i)
> diff --git a/reftable/readwrite_test.c b/reftable/readwrite_test.c
> index 5f6bcc2f775..42caf0bde4c 100644
> --- a/reftable/readwrite_test.c
> +++ b/reftable/readwrite_test.c
> @@ -254,6 +254,72 @@ static void test_log_write_read(void)
> reader_close(&rd);
> }
>
> +static void test_log_zlib_corruption(void)
> +{
> + struct reftable_write_options opts = {
> + .block_size = 256,
> + };
> + struct reftable_iterator it = { NULL };
> + struct reftable_reader rd = { NULL };
> + struct reftable_block_source source = { NULL };Nit: It doesn't matter for semantics, but usually we use "{ 0 }", and
your 1/11 does too. Would be better to do that here for consistency.> + for (i = 0; i < sizeof(message)-1; i++) {
> + message[i] = (uint8_t)(rand() % 64 + ' ');
> + }style: braces not needede.