fetch: free "raw" string when shrinking refspec

The "--prefetch" option to git-fetch modifies the default refspec,
including eliminating some entries entirely. When we drop an entry we
free the strings in the refspec_item, but we forgot to free the matching
string in the "raw" array of the refspec struct. There's no behavioral
bug here (since we correctly shrink the raw array, too), but we're
leaking the allocated string.

Let's add in the leak-fix, and while we're at it drop "const" from
the type of the raw string array. These strings are always allocated by
refspec_append(), etc, and this makes the memory ownership more clear.

This is all a bit more intimate with the refspec code than I'd like, and
I suspect it would be better if each refspec_item held on to its own raw
string, we had a single array, and we could use refspec_item_clear() to
clean up everything. But that's a non-trivial refactoring, since
refspec_item structs can be held outside of a "struct refspec", without
having a matching raw string at all. So let's leave that for now and
just fix the leak in the most immediate way.

This lets us mark t5582 as leak-free.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/fetch.c b/builtin/fetch.c
index c900f577219109473202ad032955cd3aa07ce647..80a64d0d269ed60708d7533977f9ba5c51a4b084 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -456,6 +456,7 @@ static void filter_prefetch_refspec(struct refspec *rs)
 
                        free(rs->items[i].src);
                        free(rs->items[i].dst);
+                       free(rs->raw[i]);
 
                        for (j = i + 1; j < rs->nr; j++) {
                                rs->items[j - 1] = rs->items[j];

diff --git a/refspec.c b/refspec.c
index ec90ab349a2914528a78ba73cac1dd1e344a1f6a..c3cf003443dd362390185fe471ed889e66b8dd1a 100644 (file)
--- a/refspec.c
+++ b/refspec.c
@@ -225,7 +225,7 @@ void refspec_clear(struct refspec *rs)
        rs->nr = 0;
 
        for (i = 0; i < rs->raw_nr; i++)
-               free((char *)rs->raw[i]);
+               free(rs->raw[i]);
        FREE_AND_NULL(rs->raw);
        rs->raw_alloc = 0;
        rs->raw_nr = 0;

diff --git a/refspec.h b/refspec.h
index 754be45cee3ce506f203a9fef957427210fe6536..3760fdaf2bb1833b0314adf8e66265150c74ccce 100644 (file)
--- a/refspec.h
+++ b/refspec.h
@@ -43,7 +43,7 @@ struct refspec {
        int alloc;
        int nr;
 
-       const char **raw;
+       char **raw;
        int raw_alloc;
        int raw_nr;
 

diff --git a/t/t5582-fetch-negative-refspec.sh b/t/t5582-fetch-negative-refspec.sh
index 7a80e47c2b70a8cf64c71fa00289b5abfc158151..7fa54a4029ad4443ee78ff9de44068c637feeec8 100755 (executable)
--- a/t/t5582-fetch-negative-refspec.sh
+++ b/t/t5582-fetch-negative-refspec.sh
@@ -8,6 +8,7 @@ test_description='"git fetch" with negative refspecs.
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 
 test_expect_success setup '