Skip to content

Commit

Permalink
maintenance: fix SEGFAULT when no repository
Browse files Browse the repository at this point in the history
The "git maintenance run" and "git maintenance start/stop" commands
holds a file-based lock at the .git/maintenance.lock and
.git/schedule.lock respectively. These locks are used to ensure only
one maintenance process is executed at the time as both operations
involves writing data into the git repository.

The path to the lock file is built using
"the_repository->objects->odb->path" that results in SEGFAULT when we
have no repository available as "the_repository->objects->odb" is
set to NULL.

Let's teach maintenance command to use RUN_SETUP option that will
provide the validation and fail when running outside of a repository.
Hence fixing the SEGFAULT for all three operations and making the
behaviour consistent across all subcommands.

Setting the RUN_SETUP also provides the same protection for all
subcommands given that the "register" and "unregister" also requires to
be executed inside a repository.

Furthermore let's remove the local validation implemented by the
"register" and "unregister" as this will not be required anymore with
the new option.

Signed-off-by: Rafael Silva <rafaeloliveira.cs@gmail.com>
Reviewed-by: Derrick Stolee <dstolee@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
raffs authored and gitster committed Nov 30, 2020
1 parent e67fbf9 commit e72f7de
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 8 deletions.
7 changes: 0 additions & 7 deletions builtin/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -1446,10 +1446,6 @@ static int maintenance_register(void)
struct child_process config_set = CHILD_PROCESS_INIT;
struct child_process config_get = CHILD_PROCESS_INIT;

/* There is no current repository, so skip registering it */
if (!the_repository || !the_repository->gitdir)
return 0;

/* Disable foreground maintenance */
git_config_set("maintenance.auto", "false");

Expand Down Expand Up @@ -1486,9 +1482,6 @@ static int maintenance_unregister(void)
{
struct child_process config_unset = CHILD_PROCESS_INIT;

if (!the_repository || !the_repository->gitdir)
return error(_("no current repository to unregister"));

config_unset.git_cmd = 1;
strvec_pushl(&config_unset.args, "config", "--global", "--unset",
"maintenance.repo",
Expand Down
2 changes: 1 addition & 1 deletion git.c
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ static struct cmd_struct commands[] = {
{ "ls-tree", cmd_ls_tree, RUN_SETUP },
{ "mailinfo", cmd_mailinfo, RUN_SETUP_GENTLY | NO_PARSEOPT },
{ "mailsplit", cmd_mailsplit, NO_PARSEOPT },
{ "maintenance", cmd_maintenance, RUN_SETUP_GENTLY | NO_PARSEOPT },
{ "maintenance", cmd_maintenance, RUN_SETUP | NO_PARSEOPT },
{ "merge", cmd_merge, RUN_SETUP | NEED_WORK_TREE },
{ "merge-base", cmd_merge_base, RUN_SETUP },
{ "merge-file", cmd_merge_file, RUN_SETUP_GENTLY },
Expand Down
8 changes: 8 additions & 0 deletions t/t7900-maintenance.sh
Original file line number Diff line number Diff line change
Expand Up @@ -441,4 +441,12 @@ test_expect_success 'register preserves existing strategy' '
test_config maintenance.strategy incremental
'

test_execpt_success 'fails when running outside of a repository' '
nongit test_must_fail git maintenance run &&
nongit test_must_fail git maintenance stop &&
nongit test_must_fail git maintenance start &&
nongit test_must_fail git maintenance register &&
nongit test_must_fail git maintenance unregister
'

test_done

0 comments on commit e72f7de

Please sign in to comment.