Re: [PATCH 6/6] pack-bitmap: introduce `bitmap_writer_free()`
From: Patrick Steinhardt <hidden>
Date: 2024-05-15 09:05:22
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.
quoted hunk ↗ jump to hunk
Signed-off-by: Taylor Blau <redacted> --- 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]
quoted hunk ↗ jump to hunk
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
Attachments
- signature.asc [application/pgp-signature] 833 bytes