Skip to content

Commit 2594747

Browse files
peffgitster
authored andcommitted
transport: plug leaks in transport_color_config()
We retrieve config values with repo_config_get_string(), which will allocate a new copy of the string for us. But we don't hold on to those strings, since they are just fed to git_config_colorbool() and color_parse(). But nor do we free them, which means they leak. We can fix this by using the "_tmp" form of repo_config_get_string(), which just hands us a pointer directly to the internal storage. This is OK for our purposes, since we don't need it to last for longer than our parsing calls. Two interesting side notes here: 1. Many types already have a repo_config_get_X() variant that handles this for us (e.g., repo_config_get_bool()). But neither colorbools nor colors themselves have such helpers. We might think about adding them, but converting all callers is a larger task, and out of scope for this fix. 2. As far as I can tell, this leak has been there since 960786e (push: colorize errors, 2018-04-21), but wasn't detected by LSan in our test suite. It started triggering when we applied dd3693e (transport-helper, connect: use clean_on_exit to reap children on abnormal exit, 2026-03-12) which is mostly unrelated. Even weirder, it seems to trigger only with clang (and not gcc), and only with GIT_TEST_DEFAULT_REF_FORMAT=reftable. So I think this is another odd case where the pointers happened to be hanging around in stack memory, but changing the pattern of function calls in nearby code was enough for them to be incidentally overwritten. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent c44beea commit 2594747

1 file changed

Lines changed: 3 additions & 3 deletions

File tree

transport.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,21 +47,21 @@ static int transport_color_config(void)
4747
"color.transport.reset",
4848
"color.transport.rejected"
4949
}, *key = "color.transport";
50-
char *value;
50+
const char *value;
5151
static int initialized;
5252

5353
if (initialized)
5454
return 0;
5555
initialized = 1;
5656

57-
if (!repo_config_get_string(the_repository, key, &value))
57+
if (!repo_config_get_string_tmp(the_repository, key, &value))
5858
transport_use_color = git_config_colorbool(key, value);
5959

6060
if (!want_color_stderr(transport_use_color))
6161
return 0;
6262

6363
for (size_t i = 0; i < ARRAY_SIZE(keys); i++)
64-
if (!repo_config_get_string(the_repository, keys[i], &value)) {
64+
if (!repo_config_get_string_tmp(the_repository, keys[i], &value)) {
6565
if (!value)
6666
return config_error_nonbool(keys[i]);
6767
if (color_parse(value, transport_colors[i]) < 0)

0 commit comments

Comments
 (0)