development/git
development/git
1
0
Fork 0
mirror of https://github.com/git/git synced 2024-06-30 22:54:27 +00:00
Code Issues Projects Releases Wiki Activity

sparse-checkout: free duplicate hashmap entries

Browse Source 
In insert_recursive_pattern(), we create a new pattern_entry to insert
into the parent_hashmap. If we find that the same entry already exists
in the hashmap, we skip adding the new one. But we forget to free the new
one, creating a leak.

We can fix it by cleaning up the discarded entry. It would probably be
possible to avoid creating it in the first place, but it's non-trivial.
We'd have to define a "keydata" struct that lets us compare the existing
entries to the broken-out fields. It's probably not worth the
complexity, so we'll punt on that for now.

There is one subtlety here: our insertion is happening in a loop, with
each iteration looking at the pattern we just inserted (hence the
"recursive" in the name). So if we skip insertion, what do we look at?

The obvious answer is that we should remember the existing duplicate we
found and use that. But I _think_ in that case, we probably already have
all of the recursive bits already (from when the original entry was
added). And so just breaking out of the loop would be correct. But I'm
not 100% sure on that; after all, the original leaky code could have
done the same break, but it didn't.

So I went with the "obvious answer" above, which has no chance of
changing the behavior aside from fixing the leak.

With this patch, t1091 can now be marked leak-free.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Jeff King 2024-06-04 06:13:40 -04:00 committed by Junio C Hamano
parent a544b7da2c
commit 6d107751b2
2 changed files with 9 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
builtin/sparse-checkout.c
View File

@ -523,6 +523,7 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat
char *slash = strrchr(e->pattern, '/');
char *oldpattern = e->pattern;
size_t newlen;
struct pattern_entry *dup;
if (!slash || slash == e->pattern)
break;
@ -533,8 +534,14 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat
e->pattern = xstrndup(oldpattern, newlen);
hashmap_entry_init(&e->ent, fspathhash(e->pattern));
if (!hashmap_get_entry(&pl->parent_hashmap, e, ent, NULL))
dup = hashmap_get_entry(&pl->parent_hashmap, e, ent, NULL);
if (!dup) {
hashmap_add(&pl->parent_hashmap, &e->ent);
} else {
free(e->pattern);
free(e);
e = dup;
}
}
}

1
t/t1091-sparse-checkout-builtin.sh
View File

@ -8,6 +8,7 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
GIT_TEST_SPLIT_INDEX=false
export GIT_TEST_SPLIT_INDEX
TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
list_files() {