On Sun, May 11, 2025 at 04:16:04PM +0000, Lidong Yan via GitGitGadget wrote:
> From: Lidong Yan <502024330056@xxxxxxxxxxxxxxxx>
>
> In reftable/writer.c:padded_write(), if w->writer failed, zeroed
> allocated in `reftable_calloc` will leak. w->writer could be
> `reftable_write_data` in reftable/stack.c, and could fail due to
> some write error. Simply add reftable_free(zeroed) will solve this
> problem.
>
> In reftable/writer.c:writer_index_hash(), if `reftable_buf_add` failed,
> key allocated by `reftable_malloc` will not be insert into `obj_index_tree`
> thus leaks. Simple add reftable_free(key) will solve this problem.
Nit: I think it would be sensible to split these up into two commits, as
they touch different areas of the code.
> diff --git a/reftable/writer.c b/reftable/writer.c
> index cb16f71be49..3b4ebdd6dce 100644
> --- a/reftable/writer.c
> +++ b/reftable/writer.c
> @@ -57,8 +57,10 @@ static int padded_write(struct reftable_writer *w, uint8_t *data, size_t len,
> return -1;
>
> n = w->write(w->write_arg, zeroed, w->pending_padding);
> - if (n < 0)
> + if (n < 0) {
> + reftable_free(zeroed);
> return n;
> + }
>
> w->pending_padding = 0;
> reftable_free(zeroed);
Makes sense.
> @@ -256,8 +258,10 @@ static int writer_index_hash(struct reftable_writer *w, struct reftable_buf *has
>
> reftable_buf_reset(&key->hash);
> err = reftable_buf_add(&key->hash, hash->buf, hash->len);
> - if (err < 0)
> + if (err < 0) {
> + reftable_free(key);
> return err;
> + }
> tree_insert(&w->obj_index_tree, key,
> &obj_index_tree_node_compare);
> } else {
Makes sense, as well. We want to add the node to the object index tree,
but if a step before fails then we naturally want to free the node.
Patrick
