Skip to content

Commit

Permalink
scalar: avoid segfault in reconfigure --all
Browse files Browse the repository at this point in the history
During the latest v2.45.0 update, 'scalar reconfigure --all' started to
segfault on my machine. Breaking it down via the debugger, it was
faulting on a NULL reference to the_hash_algo, which is a macro pointing
to the_repository->hash_algo.

This NULL reference appears to be due to the way the loop is abusing the
the_repository pointer, pointing it to a local repository struct after
discovering that the current directory is a valid Git repository. This
repo-swapping bit was in the original implementation from 4582676
(scalar: teach 'reconfigure' to optionally handle all registered
enlistments, 2021-12-03), but only recently started segfaulting while
trying to parse the HEAD reference. This also only happens on the
_second_ repository in the list, so does not reproduce if there is only
one registered repo.

My first inclination was to try to refactor cmd_reconfigure() to execute
'git for-each-repo' instead of this loop. In addition to the difficulty
of executing 'scalar reconfigure' within 'git for-each-repo', it would
be difficult to perform the clean-up logic for non-existent repos if we
relied on that child process.

Instead, I chose to move the temporary repo to be within the loop and
reinstate the_repository to its old value after we are done performing
logic on the current array item.

Add a test to t9210-scalar.sh to test 'scalar reconfigure --all' with
multiple registered repos, as a precaution. Unfortunately, I was unable
to reproduce the segfault using this test, so there is some coverage
left to be desired. What exactly causes my setup to hit this bug but not
this test structure? Unclear.

Signed-off-by: Derrick Stolee <stolee@gmail.com>
  • Loading branch information
derrickstolee committed Apr 30, 2024
1 parent e326e52 commit fd3f2cd
Showing 1 changed file with 17 additions and 0 deletions.
17 changes: 17 additions & 0 deletions t/t9210-scalar.sh
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,23 @@ test_expect_success 'scalar reconfigure' '
test true = "$(git -C one/src config core.preloadIndex)"
'

test_expect_success 'scalar reconfigure --all' '
repos="two three four" &&
for num in $repos
do
git init $num/src &&
scalar register $num/src &&
git -C $num/src config core.preloadIndex false || return 1
done &&
scalar reconfigure --all &&
for num in $repos
do
test true = "$(git -C $num/src config core.preloadIndex)" || return 1
done
'

test_expect_success '`reconfigure -a` removes stale config entries' '
git init stale/src &&
scalar register stale &&
Expand Down

0 comments on commit fd3f2cd

Please sign in to comment.