Skip to content

Commit 9cea034

Browse files
committed
builtin/gc: fix crash when running git maintenance start (#5198)
> This patch was sent upstream by Patrick. I'm contributing it to Git for Windows quickly to make sure it gets into microsoft/git, but also in advance of any potential 2.47.1. It was reported on the mailing list that running `git maintenance start` immediately segfaults starting with b6c3f8e (builtin/maintenance: fix leak in `get_schedule_cmd()`, 2024-09-26). And indeed, this segfault is trivial to reproduce up to a point where one is scratching their head why we didn't catch this regression in our test suite. The root cause of this error is `get_schedule_cmd()`, which does not populate the `out` parameter in all cases anymore starting with the mentioned commit. Callers do assume it to always be populated though and will e.g. call `strvec_split()` on the returned value, which will of course segfault when the variable is uninitialized. So why didn't we catch this trivial regression? The reason is that our tests always set up the "GIT_TEST_MAINT_SCHEDULER" environment variable via "t/test-lib.sh", which allows us to override the scheduler command with a custom one so that we don't accidentally modify the developer's system. But the faulty code where we don't set the `out` parameter will only get hit in case that environment variable is _not_ set, which is never the case when executing our tests. Fix the regression by again unconditionally allocating the value in the `out` parameter, if provided. Add a test that unsets the environment variable to catch future regressions in this area.
2 parents b8a3eb4 + 59beef8 commit 9cea034

File tree

2 files changed

+7
-1
lines changed

2 files changed

+7
-1
lines changed

t/t7900-maintenance.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -646,7 +646,7 @@ test_expect_success !MINGW 'register and unregister with regex metacharacters' '
646646
maintenance.repo "$(pwd)/$META"
647647
'
648648

649-
test_expect_success 'start without GIT_TEST_MAINT_SCHEDULER' '
649+
test_expect_success !MINGW,!DARWIN 'start without GIT_TEST_MAINT_SCHEDULER' '
650650
test_when_finished "rm -rf crontab.log script repo" &&
651651
mkdir script &&
652652
write_script script/crontab <<-EOF &&

t/test-lib.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1738,6 +1738,12 @@ case $uname_s in
17381738
test_set_prereq GREP_STRIPS_CR
17391739
test_set_prereq WINDOWS
17401740
;;
1741+
*Darwin*)
1742+
test_set_prereq POSIXPERM
1743+
test_set_prereq BSLASHPSPEC
1744+
test_set_prereq EXECKEEPSPID
1745+
test_set_prereq DARWIN
1746+
;;
17411747
*)
17421748
test_set_prereq POSIXPERM
17431749
test_set_prereq BSLASHPSPEC

0 commit comments

Comments
 (0)