sparse-checkout: pass string literals directly to add_pattern()

The add_pattern() function takes a pattern string, but neither makes a copy of it nor takes ownership of the memory. So it is the caller's responsibility to make sure the string hangs around as long as the pattern_list which references it. There are a few cases in sparse-checkout where we use string literal patterns by stuffing them into a strbuf, detaching the buffer, and then passing the result into add_pattern(). This creates a leak when the pattern_list is eventually cleared, since we don't retain a copy of the detached buffer to free. But we can observe that the whole strbuf dance is unnecessary. The point was presumably[1] to satisfy the lifetime requirement of the string. But string literals have static duration; we can count on them lasting for the whole program. So we can fix the leak by just passing them directly. And as a bonus, that simplifies the code. The leaks can be seen in t7002, which drops from 25 leaks to 22 with this patch. It also makes t3602 and t1090 leak-free. In the long run, we will also want to clean up this (undocumented!) memory lifetime requirement of add_pattern(). But that can come in a later patch; passing the string literals directly will be the right thing either way. [1] The code in question comes from 416adc8711 (sparse-checkout: update working directory in-process for 'init', 2019-11-21) and 99dfa6f970 (sparse-checkout: use in-process update for disable subcommand, 2019-11-21), but I didn't see anything in their commit messages or on the list explaining the strbufs. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jun 4, 2024 at 06:13 UTC 4d7f95ed1f5c4a8842f8a9f9614ee0c66c85bfbd
3 files changed +5 -8
builtin/sparse-checkout.c
+3 -8
@@ -442,7 +442,6 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix)
442 char *sparse_filename;
443 int res;
444 struct object_id oid;
445 - struct strbuf pattern = STRBUF_INIT;
445
446 static struct option builtin_sparse_checkout_init_options[] = {
447 OPT_BOOL(0, "cone", &init_opts.cone_mode,
@@ -493,10 +492,8 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix)
492 return 0;
493 }
494
496 - strbuf_addstr(&pattern, "/*");
497 - add_pattern(strbuf_detach(&pattern, NULL), empty_base, 0, &pl, 0);
498 - strbuf_addstr(&pattern, "!/*/");
499 - add_pattern(strbuf_detach(&pattern, NULL), empty_base, 0, &pl, 0);
495 + add_pattern("/*", empty_base, 0, &pl, 0);
496 + add_pattern("!/*/", empty_base, 0, &pl, 0);
497 pl.use_cone_patterns = init_opts.cone_mode;
498
499 return write_patterns_and_update(&pl);
@@ -893,7 +890,6 @@ static int sparse_checkout_disable(int argc, const char **argv,
890 OPT_END(),
891 };
892 struct pattern_list pl;
896 - struct strbuf match_all = STRBUF_INIT;
893
894 /*
895 * We do not exit early if !core_apply_sparse_checkout; due to the
@@ -919,8 +915,7 @@ static int sparse_checkout_disable(int argc, const char **argv,
915 pl.use_cone_patterns = 0;
916 core_apply_sparse_checkout = 1;
917
922 - strbuf_addstr(&match_all, "/*");
923 - add_pattern(strbuf_detach(&match_all, NULL), empty_base, 0, &pl, 0);
918 + add_pattern("/*", empty_base, 0, &pl, 0);
919
920 prepare_repo_settings(the_repository);
921 the_repository->settings.sparse_index = 0;
t/t1090-sparse-checkout-scope.sh
+1
@@ -6,6 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
6 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
7
8 TEST_CREATE_REPO_NO_TEMPLATE=1
9 +TEST_PASSES_SANITIZE_LEAK=true
10 . ./test-lib.sh
11
12 test_expect_success 'setup' '
t/t3602-rm-sparse-checkout.sh
+1
@@ -2,6 +2,7 @@
2
3 test_description='git rm in sparse checked out working trees'
4
5 +TEST_PASSES_SANITIZE_LEAK=true
6 . ./test-lib.sh
7
8 test_expect_success 'setup' "