On Thu, May 30, 2024 at 09:24:34AM +0200, Patrick Steinhardt wrote:
> > Looks like no. The obvious candidate would be t7002-mv-sparse-checkout,
> > but it looks like the sparse-checkout code has minor leaks itself.
>
> Okay, thanks for double checking! I was mostly asking because I plan to
> send another leak fixes series to the mailing list later this week.
OK, good news. t7002 _does_ trigger the leak fixed in my patch. You just
can't tell because of all of the sparse-checkout leaks. ;)
The (messy) patch below gets it to a leak-free state when applied on
top. Do you want me to do another mini-series with it, or do you want to
just roll it into what you're doing (I won't be surprised if you've
already found some of these).
-Peff
diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
index 0f52e25249..1ed9dfa886 100644
--- a/builtin/sparse-checkout.c
+++ b/builtin/sparse-checkout.c
@@ -311,6 +311,8 @@ static void write_cone_to_file(FILE *fp, struct pattern_list *pl)
fprintf(fp, "%s/\n", pattern);
free(pattern);
}
+
+ string_list_clear(&sl, 0);
}
static int write_patterns_and_update(struct pattern_list *pl)
@@ -471,6 +473,7 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix)
/* If we already have a sparse-checkout file, use it. */
if (res >= 0) {
free(sparse_filename);
+ clear_pattern_list(&pl);
return update_working_directory(NULL);
}
@@ -486,6 +489,7 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix)
die(_("failed to open '%s'"), sparse_filename);
free(sparse_filename);
+ clear_pattern_list(&pl);
fprintf(fp, "/*\n!/*/\n");
fclose(fp);
return 0;
@@ -525,6 +529,10 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat
if (!hashmap_get_entry(&pl->parent_hashmap, e, ent, NULL))
hashmap_add(&pl->parent_hashmap, &e->ent);
+ else {
+ free(e->pattern);
+ free(e);
+ }
}
}
@@ -891,7 +899,6 @@ static int sparse_checkout_disable(int argc, const char **argv,
OPT_END(),
};
struct pattern_list pl;
- struct strbuf match_all = STRBUF_INIT;
/*
* We do not exit early if !core_apply_sparse_checkout; due to the
@@ -917,8 +924,7 @@ static int sparse_checkout_disable(int argc, const char **argv,
pl.use_cone_patterns = 0;
core_apply_sparse_checkout = 1;
- strbuf_addstr(&match_all, "/*");
- add_pattern(strbuf_detach(&match_all, NULL), empty_base, 0, &pl, 0);
+ add_pattern("/*", empty_base, 0, &pl, 0);
prepare_repo_settings(the_repository);
the_repository->settings.sparse_index = 0;
diff --git a/dir.c b/dir.c
index 2d83f3311a..5769c4e693 100644
--- a/dir.c
+++ b/dir.c
@@ -799,6 +799,8 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
if (given->patternlen > 2 &&
!strcmp(given->pattern + given->patternlen - 2, "/*")) {
+ struct pattern_entry *old;
+
if (!(given->flags & PATTERN_FLAG_NEGATIVE)) {
/* Not a cone pattern. */
warning(_("unrecognized pattern: '%s'"), given->pattern);
@@ -824,7 +826,11 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
}
hashmap_add(&pl->parent_hashmap, &translated->ent);
- hashmap_remove(&pl->recursive_hashmap, &translated->ent, &data);
+ old = hashmap_remove_entry(&pl->recursive_hashmap, translated, ent, &data);
+ if (old) {
+ free(old->pattern);
+ free(old);
+ }
free(data);
return;
}
@@ -855,6 +861,7 @@ static void add_pattern_to_hashsets(struct pattern_list *pl, struct path_pattern
clear_hashmaps:
warning(_("disabling cone pattern matching"));
+ /* should free ent->pattern too; refactor clear_pattern_list? */
hashmap_clear_and_free(&pl->parent_hashmap, struct pattern_entry, ent);
hashmap_clear_and_free(&pl->recursive_hashmap, struct pattern_entry, ent);
pl->use_cone_patterns = 0;
@@ -950,13 +957,20 @@ static int read_skip_worktree_file_from_index(struct index_state *istate,
*/
void clear_pattern_list(struct pattern_list *pl)
{
+ struct hashmap_iter iter;
+ struct pattern_entry *entry;
int i;
for (i = 0; i < pl->nr; i++)
free(pl->patterns[i]);
free(pl->patterns);
free(pl->filebuf);
+
+ hashmap_for_each_entry(&pl->recursive_hashmap, &iter, entry, ent)
+ free(entry->pattern);
hashmap_clear_and_free(&pl->recursive_hashmap, struct pattern_entry, ent);
+ hashmap_for_each_entry(&pl->parent_hashmap, &iter, entry, ent)
+ free(entry->pattern);
hashmap_clear_and_free(&pl->parent_hashmap, struct pattern_entry, ent);
memset(pl, 0, sizeof(*pl));
--
2.45.1.692.gbe047d9c60
