Add a function to free struct bitmap_index instances, and use it where
needed (except when rebuild_existing_bitmaps() is used, since it creates
references to the bitmaps within the struct bitmap_index passed to it).

Note that the hashes field in struct bitmap_index is not freed because
it points to another field within the same struct. The documentation for
that field has been updated to clarify that.

Signed-off-by: Jonathan Tan <jonathanta...@google.com>
---
 builtin/pack-objects.c |  1 +
 builtin/rev-list.c     |  2 ++
 pack-bitmap-write.c    |  4 ++++
 pack-bitmap.c          | 35 +++++++++++++++++++++++++++++------
 pack-bitmap.h          |  1 +
 5 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index d064f944b..896e41300 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2945,6 +2945,7 @@ static int get_object_list_from_bitmap(struct rev_info 
*revs)
        }
 
        traverse_bitmap_commit_list(bitmap_git, &add_object_entry_from_bitmap);
+       free_bitmap_index(bitmap_git);
        return 0;
 }
 
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index cce42ae1d..62776721f 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -521,6 +521,7 @@ int cmd_rev_list(int argc, const char **argv, const char 
*prefix)
                                if (max_count >= 0 && max_count < commit_count)
                                        commit_count = max_count;
                                printf("%d\n", commit_count);
+                               free_bitmap_index(bitmap_git);
                                return 0;
                        }
                } else if (revs.max_count < 0 &&
@@ -528,6 +529,7 @@ int cmd_rev_list(int argc, const char **argv, const char 
*prefix)
                        struct bitmap_index *bitmap_git;
                        if ((bitmap_git = prepare_bitmap_walk(&revs))) {
                                traverse_bitmap_commit_list(bitmap_git, 
&show_object_fast);
+                               free_bitmap_index(bitmap_git);
                                return 0;
                        }
                }
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 03e122563..7896fedd3 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -367,6 +367,10 @@ void bitmap_writer_reuse_bitmaps(struct packing_data 
*to_pack)
        writer.reused = kh_init_sha1();
        rebuild_existing_bitmaps(bitmap_git, to_pack, writer.reused,
                                 writer.show_progress);
+       /*
+        * NEEDSWORK: rebuild_existing_bitmaps() makes writer.reused reference
+        * some bitmaps in bitmap_git, so we can't free the latter.
+        */
 }
 
 static struct ewah_bitmap *find_reused_bitmap(const unsigned char *sha1)
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 7795444b0..c06b19f49 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -66,7 +66,7 @@ struct bitmap_index {
        /* Number of bitmapped commits */
        uint32_t entry_count;
 
-       /* Name-hash cache (or NULL if not present). */
+       /* If not NULL, this is a name-hash cache pointing into map. */
        uint32_t *hashes;
 
        /*
@@ -350,6 +350,7 @@ struct bitmap_index *prepare_bitmap_git(void)
        if (!open_pack_bitmap(bitmap_git) && !load_pack_bitmap(bitmap_git))
                return bitmap_git;
 
+       free_bitmap_index(bitmap_git);
        return NULL;
 }
 
@@ -690,7 +691,7 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info 
*revs)
        /* try to open a bitmapped pack, but don't parse it yet
         * because we may not need to use it */
        if (open_pack_bitmap(bitmap_git) < 0)
-               return NULL;
+               goto cleanup;
 
        for (i = 0; i < revs->pending.nr; ++i) {
                struct object *object = revs->pending.objects[i].item;
@@ -723,11 +724,11 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info 
*revs)
         * optimize here
         */
        if (haves && !in_bitmapped_pack(bitmap_git, haves))
-               return NULL;
+               goto cleanup;
 
        /* if we don't want anything, we're done here */
        if (!wants)
-               return NULL;
+               goto cleanup;
 
        /*
         * now we're going to use bitmaps, so load the actual bitmap entries
@@ -735,7 +736,7 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info 
*revs)
         * becomes invalidated and we must perform the revwalk through bitmaps
         */
        if (!bitmap_git->loaded && load_pack_bitmap(bitmap_git) < 0)
-               return NULL;
+               goto cleanup;
 
        object_array_clear(&revs->pending);
 
@@ -761,6 +762,10 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info 
*revs)
 
        bitmap_free(haves_bitmap);
        return bitmap_git;
+
+cleanup:
+       free_bitmap_index(bitmap_git);
+       return NULL;
 }
 
 int reuse_partial_packfile_from_bitmap(struct bitmap_index *bitmap_git,
@@ -1001,7 +1006,7 @@ void test_bitmap_walk(struct rev_info *revs)
        else
                fprintf(stderr, "Mismatch!\n");
 
-       bitmap_free(result);
+       free_bitmap_index(bitmap_git);
 }
 
 static int rebuild_bitmap(uint32_t *reposition,
@@ -1093,3 +1098,21 @@ int rebuild_existing_bitmaps(struct bitmap_index 
*bitmap_git,
        bitmap_free(rebuild);
        return 0;
 }
+
+void free_bitmap_index(struct bitmap_index *b)
+{
+       if (!b)
+               return;
+
+       if (b->map)
+               munmap(b->map, b->map_size);
+       ewah_pool_free(b->commits);
+       ewah_pool_free(b->trees);
+       ewah_pool_free(b->blobs);
+       ewah_pool_free(b->tags);
+       kh_destroy_sha1(b->bitmaps);
+       free(b->ext_index.objects);
+       free(b->ext_index.hashes);
+       bitmap_free(b->result);
+       free(b);
+}
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 19f70043a..4555907de 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -48,6 +48,7 @@ int reuse_partial_packfile_from_bitmap(struct bitmap_index *,
                                       uint32_t *entries, off_t *up_to);
 int rebuild_existing_bitmaps(struct bitmap_index *, struct packing_data 
*mapping,
                             khash_sha1 *reused_bitmaps, int show_progress);
+void free_bitmap_index(struct bitmap_index *);
 
 void bitmap_writer_show_progress(int show);
 void bitmap_writer_set_checksum(unsigned char *sha1);
-- 
2.17.0.768.g1526ddbba1.dirty

Reply via email to