git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Rafael Silva <rafaeloliveira.cs@gmail.com>
To: git@vger.kernel.org
Cc: "Derrick Stolee" <dstolee@microsoft.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Martin Ågren" <martin.agren@gmail.com>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Rafael Silva" <rafaeloliveira.cs@gmail.com>
Subject: [PATCH v2 1/1] maintenance: fix SEGFAULT when no repository
Date: Thu, 26 Nov 2020 20:41:41 +0000	[thread overview]
Message-ID: <20201126204141.1438-2-rafaeloliveira.cs@gmail.com> (raw)
In-Reply-To: <20201126204141.1438-1-rafaeloliveira.cs@gmail.com>

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>
---
 builtin/gc.c           | 7 -------
 git.c                  | 2 +-
 t/t7900-maintenance.sh | 8 ++++++++
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/builtin/gc.c b/builtin/gc.c
index bc25ad52c7..ebb0158308 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -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");
 
@@ -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",
diff --git a/git.c b/git.c
index 4b7bd77b80..a00a0a4d94 100644
--- a/git.c
+++ b/git.c
@@ -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 },
diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh
index d9e68bb2bf..ae5c29b0ff 100755
--- a/t/t7900-maintenance.sh
+++ b/t/t7900-maintenance.sh
@@ -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
-- 
2.29.2.367.g37477fb670


  reply	other threads:[~2020-11-26 20:53 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-24 16:44 [PATCH 0/1] maintenance: Fix a SEGFAULT when no repository when running git maintenance run/start Rafael Silva
2020-11-24 16:44 ` [PATCH 1/1] maintenance: fix a SEGFAULT when no repository Rafael Silva
2020-11-24 17:22   ` Derrick Stolee
2020-11-24 21:48     ` Junio C Hamano
2020-11-24 21:59       ` Derrick Stolee
2020-11-26  8:22     ` Rafael Silva
2020-11-26 11:21       ` Derrick Stolee
2020-11-24 17:24   ` Eric Sunshine
2020-11-24 19:14     ` SZEDER Gábor
2020-11-24 19:34       ` Eric Sunshine
2020-11-26  7:13       ` Rafael Silva
2020-11-24 19:03   ` Martin Ågren
2020-11-26  7:07     ` Rafael Silva
2020-11-26 20:41 ` [PATCH v2 0/1] maintenance: Fix SEGFAULT when running outside of a repository Rafael Silva
2020-11-26 20:41   ` Rafael Silva [this message]
2020-11-27 20:43     ` [PATCH v2 1/1] maintenance: fix SEGFAULT when no repository Derrick Stolee
2020-12-08 20:12     ` Josh Steadmon
2020-12-08 21:58       ` Junio C Hamano
2020-12-08 22:17         ` Junio C Hamano
2020-12-24  8:12           ` Christian Couder
2020-12-24 14:14             ` Junio C Hamano
2020-12-09  9:29       ` Rafael Silva

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201126204141.1438-2-rafaeloliveira.cs@gmail.com \
    --to=rafaeloliveira.cs@gmail.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=martin.agren@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=sunshine@sunshineco.com \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).