Re: [PATCH 03/16] mktree: use non-static tree_entry array

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jun 11, 2024, 18:45 UTC
Message-ID: <CAPig+cSg_SgU_ETktkz7KapLmzdDjzvjEx-_jS8L-eguBGk4oQ@mail.gmail.com>
In-Reply-To: <5ade145352f44b431c16a2ec29cd87de489e8032.1718130288.git.gitgitgadget@gmail.com>

On Tue, Jun 11, 2024 at 2:25 PM Victoria Dye via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 28 quoted lines

> Replace the static 'struct tree_entry **entries' with a non-static 'struct
> tree_entry_array' instance. In later commits, we'll want to be able to
> create additional 'struct tree_entry_array' instances utilizing common
> functionality (create, push, clear, free). To avoid code duplication, create
> the 'struct tree_entry_array' type and add functions that perform those
> basic operations.
>
> Signed-off-by: Victoria Dye <vdye@github.com>
> ---
> diff --git a/builtin/mktree.c b/builtin/mktree.c
> @@ -12,15 +12,39 @@
> +struct tree_entry_array {
> +       size_t nr, alloc;
> +       struct tree_entry **entries;
> +};
>
> +static void clear_tree_entry_array(struct tree_entry_array *arr)
> +{
> +       for (size_t i = 0; i < arr->nr; i++)
> +               FREE_AND_NULL(arr->entries[i]);
> +       arr->nr = 0;
> +}
> +
> +static void release_tree_entry_array(struct tree_entry_array *arr)
> +{
> +       FREE_AND_NULL(arr->entries);
> +       arr->nr = arr->alloc = 0;
> +}

For robustness, to make it less likely for future code to leak the items pointed to by `arr->entries`, it might make sense for release_tree_entry_array() to call clear_tree_entry_array() before calling FREE_AND_NULL().

> -       ALLOC_GROW(entries, used + 1, alloc);
> -       entries[used++] = ent;
> +       /* Append the update */
> +       tree_entry_array_push(arr, ent);

Nit: the new comment seems superfluous