[PATCH v2 0/13] leak fixes for sparse-checkout code
- From
Jeff King <peff@peff.net>
- Date
- Jun 4, 2024, 10:08 UTC
- Message-ID
- <20240604100814.GA1304520@coredump.intra.peff.net>
- In-Reply-To
- <20240531112433.GA428583@coredump.intra.peff.net>
On Fri, May 31, 2024 at 07:24:34AM -0400, Jeff King wrote:
Show 21 quoted lines
> So Patrick nerd-sniped me by asking if my earlier leakfix for git-mv was > triggered by the test suite. It was, in t7002, but that wasn't enough to > make the script leak-free. So I figured, how hard could it be to go all > the way? > > Well. It only took a few patches (1-5), but in the process I stumbled on > a rather tricky interface oddity of add_pattern(), which caused some > other leaks. The interface is fixed in patch 6, and the matching leak > goes away in patch 7. Of course, I wanted to make sure it was tested, so > after poking around I found that t1091 triggered it. > > But as you might guess, that didn't make t1091 leak-free. And I couldn't > bear leaving it on a cliffhanger like that, so patches 8-13 fix the rest > of the issues triggered by that script. > > And along the way we managed to make t1090 and t3602 leak-free, too > (actually in patch 2, but I didn't notice until the whole thing was > done). > > These should apply on top of jk/leakfixes, since the leak-freeness of > t7002 depends on the fix there.
Here's a v2 with a few minor updates, based on review from Patrick. The restoration of "old_pl" in patch 9 is now done unconditionally (it doesn't matter in practice due to a bunch of subtle things, but it's the less surprising and more defensive choice). And a small style update in patch 13.
Range-diff:
1: 1be529905f = 1: 1be529905f sparse-checkout: free string list in write_cone_to_file()
2: 8e36bf2213 = 2: 8e36bf2213 sparse-checkout: pass string literals directly to add_pattern()
3: 1a3c5819f9 = 3: 1a3c5819f9 dir.c: free strings in sparse cone pattern hashmaps
4: a492842dd3 = 4: a492842dd3 sparse-checkout: clear patterns when init() sees existing sparse file
5: 2d8d402809 = 5: 2d8d402809 dir.c: free removed sparse-pattern hashmap entries
6: addb69e229 = 6: addb69e229 dir.c: always copy input to add_pattern()
7: f2ee2192f7 = 7: f2ee2192f7 sparse-checkout: reuse --stdin buffer when reading patterns
8: d3f7ea12ab = 8: d3f7ea12ab sparse-checkout: always free "line" strbuf after reading input
9: b5c07325d2 ! 9: b78d3b51b9 sparse-checkout: refactor temporary sparse_checkout_patterns
@@ builtin/sparse-checkout.c: static int update_working_directory(struct pattern_li
+ if (r->index->sparse_checkout_patterns != pl) {
+ clear_pattern_list(r->index->sparse_checkout_patterns);
+ FREE_AND_NULL(r->index->sparse_checkout_patterns);
-+ } else {
-+ r->index->sparse_checkout_patterns = old_pl;
+ }
++ r->index->sparse_checkout_patterns = old_pl;
++
return result;
}
10: 26b1e08e6f = 10: ef05901aef sparse-checkout: free sparse_filename after use
11: d14f0f0546 = 11: 7b423b1691 sparse-checkout: free pattern list in sparse_checkout_list()
12: 84b1a5eb0d = 12: 034668997b sparse-checkout: free string list after displaying
13: 460a7f9324 ! 13: 82f14304ae sparse-checkout: free duplicate hashmap entries
@@ builtin/sparse-checkout.c: static void insert_recursive_pattern(struct pattern_l
- if (!hashmap_get_entry(&pl->parent_hashmap, e, ent, NULL))
+ dup = hashmap_get_entry(&pl->parent_hashmap, e, ent, NULL);
-+ if (!dup)
++ if (!dup) {
hashmap_add(&pl->parent_hashmap, &e->ent);
-+ else {
++ } else {
+ free(e->pattern);
+ free(e);
+ e = dup;[01/13]: sparse-checkout: free string list in write_cone_to_file() [02/13]: sparse-checkout: pass string literals directly to add_pattern() [03/13]: dir.c: free strings in sparse cone pattern hashmaps [04/13]: sparse-checkout: clear patterns when init() sees existing sparse file [05/13]: dir.c: free removed sparse-pattern hashmap entries [06/13]: dir.c: always copy input to add_pattern() [07/13]: sparse-checkout: reuse --stdin buffer when reading patterns [08/13]: sparse-checkout: always free "line" strbuf after reading input [09/13]: sparse-checkout: refactor temporary sparse_checkout_patterns [10/13]: sparse-checkout: free sparse_filename after use [11/13]: sparse-checkout: free pattern list in sparse_checkout_list() [12/13]: sparse-checkout: free string list after displaying [13/13]: sparse-checkout: free duplicate hashmap entries
builtin/sparse-checkout.c | 49 +++++++++++++++++++----------- dir.c | 42 ++++++++++++++++--------- dir.h | 3 +- t/t1090-sparse-checkout-scope.sh | 1 + t/t1091-sparse-checkout-builtin.sh | 1 + t/t3602-rm-sparse-checkout.sh | 1 + t/t7002-mv-sparse-checkout.sh | 1 + 7 files changed, 65 insertions(+), 33 deletions(-)
-Peff