On Tue, May 14, 2024 at 03:57:06PM -0400, Taylor Blau wrote:
> Now that there is clearer memory ownership around the bitmap_writer
> structure, introduce a bitmap_writer_free() function that callers may
> use to free any memory associated with their instance of the
> bitmap_writer structure.
Great. I wanted to ask about this in preceding commits already, good to
see that you already thought of if.
> Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
> ---
> builtin/pack-objects.c | 3 ++-
> midx-write.c | 1 +
> pack-bitmap-write.c | 23 +++++++++++++++++++++++
> pack-bitmap.h | 1 +
> 4 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index 10e69fdc8e..26a6d0d791 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -1245,7 +1245,6 @@ static void write_pack_file(void)
> uint32_t nr_remaining = nr_result;
> time_t last_mtime = 0;
> struct object_entry **write_order;
> - struct bitmap_writer bitmap_writer;
>
> if (progress > pack_to_stdout)
> progress_state = start_progress(_("Writing objects"), nr_result);
> @@ -1315,6 +1314,7 @@ static void write_pack_file(void)
> if (!pack_to_stdout) {
> struct stat st;
> struct strbuf tmpname = STRBUF_INIT;
> + struct bitmap_writer bitmap_writer;
> char *idx_tmp_name = NULL;
>
> /*
Nit: we could have avoided moving the struct if it was introduced in
this spot in the preceding patch.
[snip]
> diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
> index 420f17c2e0..6cae670412 100644
> --- a/pack-bitmap-write.c
> +++ b/pack-bitmap-write.c
> @@ -32,6 +32,29 @@ void bitmap_writer_init(struct bitmap_writer *writer)
> memset(writer, 0, sizeof(struct bitmap_writer));
> }
>
> +void bitmap_writer_free(struct bitmap_writer *writer)
> +{
> + uint32_t i;
> +
> + if (!writer)
> + return;
> +
> + ewah_free(writer->commits);
> + ewah_free(writer->trees);
> + ewah_free(writer->blobs);
> + ewah_free(writer->tags);
> +
> + kh_destroy_oid_map(writer->bitmaps);
> +
> + for (i = 0; i < writer->selected_nr; i++) {
> + struct bitmapped_commit *bc = &writer->selected[i];
> + if (bc->write_as != bc->bitmap)
> + ewah_free(bc->write_as);
> + ewah_free(bc->bitmap);
> + }
> + free(writer->selected);
I was wondering whether we also want to set all those fields to `NULL`
at the end, or just `memset()` the whole structure. But there probably
isn't much of a reason to do it currently, so I don't mind if your
answer is "no".
Patrick
Attachment:
signature.asc
Description: PGP signature
