# Re: \[PATCH v3 02/11\] reftable: fix resource leak in error path

- From: Ævar Arnfjörð Bjarmason \<hidden\>
- Date: 2021-12-13 16:22:02 
- List: git
- Page: https://ratatoskr.run/git/2021/12/7915640
- Thread: https://ratatoskr.run/git/2021/12/7914155/t.md

```text

On Mon, Dec 13 2021, Han-Wen Nienhuys via GitGitGadget wrote:

> From: Han-Wen Nienhuys <redacted>
> [...]
> -	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...

>  
>  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.
```