Skip to content

Commit 4d7f95e

Browse files
peffgitster
authored andcommitted
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 416adc8 (sparse-checkout: update working directory in-process for 'init', 2019-11-21) and 99dfa6f (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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 2181fe6 commit 4d7f95e

File tree

3 files changed

+5
-8
lines changed

3 files changed

+5
-8
lines changed

builtin/sparse-checkout.c

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,6 @@ static int sparse_checkout_init(int argc, const char **argv, const char *prefix)
442442
char *sparse_filename;
443443
int res;
444444
struct object_id oid;
445-
struct strbuf pattern = STRBUF_INIT;
446445

447446
static struct option builtin_sparse_checkout_init_options[] = {
448447
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)
493492
return 0;
494493
}
495494

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);
500497
pl.use_cone_patterns = init_opts.cone_mode;
501498

502499
return write_patterns_and_update(&pl);
@@ -893,7 +890,6 @@ static int sparse_checkout_disable(int argc, const char **argv,
893890
OPT_END(),
894891
};
895892
struct pattern_list pl;
896-
struct strbuf match_all = STRBUF_INIT;
897893

898894
/*
899895
* 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,
919915
pl.use_cone_patterns = 0;
920916
core_apply_sparse_checkout = 1;
921917

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);
924919

925920
prepare_repo_settings(the_repository);
926921
the_repository->settings.sparse_index = 0;

t/t1090-sparse-checkout-scope.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
66
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
77

88
TEST_CREATE_REPO_NO_TEMPLATE=1
9+
TEST_PASSES_SANITIZE_LEAK=true
910
. ./test-lib.sh
1011

1112
test_expect_success 'setup' '

t/t3602-rm-sparse-checkout.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
test_description='git rm in sparse checked out working trees'
44

5+
TEST_PASSES_SANITIZE_LEAK=true
56
. ./test-lib.sh
67

78
test_expect_success 'setup' "

0 commit comments

Comments
 (0)