bulk-checkin: fix leaking state TODO

When flushing a bulk-checking to disk we also reset the `struct
bulk_checkin_packfile` state. But while we free some of its members,
others aren't being free'd, leading to memory leaks:

  - The temporary packfile name is not getting freed.

  - The `struct hashfile` only gets freed in case we end up calling
    `finalize_hashfile()`. There are code paths though where that is not
    the case, namely when nothing has been written. For this, we need to
    make `free_hashfile()` public.

Fix those leaks.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/bulk-checkin.c b/bulk-checkin.c
index da8673199b28f5641181432d890e5b331bebdd67..9089c214fa4e8f6d2120fe0d6e9dca262bcdbe76 100644 (file)
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -61,6 +61,7 @@ static void flush_bulk_checkin_packfile(struct bulk_checkin_packfile *state)
 
        if (state->nr_written == 0) {
                close(state->f->fd);
+               free_hashfile(state->f);
                unlink(state->pack_tmp_name);
                goto clear_exit;
        } else if (state->nr_written == 1) {
@@ -83,6 +84,7 @@ static void flush_bulk_checkin_packfile(struct bulk_checkin_packfile *state)
                free(state->written[i]);
 
 clear_exit:
+       free(state->pack_tmp_name);
        free(state->written);
        memset(state, 0, sizeof(*state));
 

diff --git a/csum-file.c b/csum-file.c
index 8abbf013250eef0d5bc33cf2f47ce6797c51e0bb..7e0ece1305a7574918b84df5f90d2f84b05fe5d8 100644 (file)
--- a/csum-file.c
+++ b/csum-file.c
@@ -56,7 +56,7 @@ void hashflush(struct hashfile *f)
        }
 }
 
-static void free_hashfile(struct hashfile *f)
+void free_hashfile(struct hashfile *f)
 {
        free(f->buffer);
        free(f->check_buffer);

diff --git a/csum-file.h b/csum-file.h
index 566e05cbd25a04be9977356e3cfb5a77637bf4c8..ca553eba173cd1c004329d4600716b7768c8379e 100644 (file)
--- a/csum-file.h
+++ b/csum-file.h
@@ -46,6 +46,16 @@ int hashfile_truncate(struct hashfile *, struct hashfile_checkpoint *);
 struct hashfile *hashfd(int fd, const char *name);
 struct hashfile *hashfd_check(const char *name);
 struct hashfile *hashfd_throughput(int fd, const char *name, struct progress *tp);
+
+/*
+ * Free the hashfile without flushing its contents to disk. This only
+ * needs to be called when not calling `finalize_hashfile()`.
+ */
+void free_hashfile(struct hashfile *f);
+
+/*
+ * Finalize the hashfile by flushing data to disk and free'ing it.
+ */
 int finalize_hashfile(struct hashfile *, unsigned char *, enum fsync_component, unsigned int);
 void hashwrite(struct hashfile *, const void *, unsigned int);
 void hashflush(struct hashfile *f);

diff --git a/t/t1050-large.sh b/t/t1050-large.sh
index c71932b02423734908428a9b2be776fec229ce9e..ed638f664479bd1abfca19b985aeaaff66452f18 100755 (executable)
--- a/t/t1050-large.sh
+++ b/t/t1050-large.sh
@@ -3,6 +3,7 @@
 
 test_description='adding and checking out large blobs'
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success 'core.bigFileThreshold must be non-negative' '