[PATCH] pager: fix crash when pager program doesn't exist

[Enzo Matsumiya <ematsumiya@suse.de>]

setup_pager() doesn't properly free pager_process.argv if
start_command() fails, nor finish_command() is ever called.

setup_pager() is called twice, once from commit_pager_choice(), and then
from cmd_log_init_finish(). On the first run, it runs fine because
start_command() assigns cmd->args.v to cmd->argv, and upon command
failure, child_process_clear() clears cmd->args.

On the second run, though, argv is no longer NULL, but .args has been
cleared, so any strvec_push() operation will crash.

This patch makes sure that freeing pager_process.argv is part of its
argument preparation, as well as zeroing the whole pager_process struct.

Reproduction:
$ git config pager.show INVALID_PAGER
$ git show $VALID_COMMIT
error: cannot run INVALID_PAGER: No such file or directory
[1]    3619 segmentation fault (core dumped)  git show $VALID_COMMIT

While at it, I implemented a fallback to the DEFAULT_PAGER, so it tries
at least one more time when the configured pager fails.

Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
 pager.c       | 25 +++++++++++++++++++++----
 run-command.c |  1 +
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/pager.c b/pager.c
index 52f27a6765c8..79a47db55d63 100644
--- a/pager.c
+++ b/pager.c
@@ -97,6 +97,9 @@ static void setup_pager_env(struct strvec *env)
 
 void prepare_pager_args(struct child_process *pager_process, const char *pager)
 {
+	child_process_clear(pager_process);
+	if (pager_process->argv)
+		free(pager_process->argv);
 	strvec_push(&pager_process->args, pager);
 	pager_process->use_shell = 1;
 	setup_pager_env(&pager_process->env_array);
@@ -105,11 +108,14 @@ void prepare_pager_args(struct child_process *pager_process, const char *pager)
 
 void setup_pager(void)
 {
-	const char *pager = git_pager(isatty(1));
+	const char *tmp_pager = git_pager(isatty(1));
+	char *pager;
 
-	if (!pager)
+	if (!tmp_pager)
 		return;
 
+	pager = xstrdup(tmp_pager);
+
 	/*
 	 * After we redirect standard output, we won't be able to use an ioctl
 	 * to get the terminal size. Let's grab it now, and then set $COLUMNS
@@ -124,12 +130,23 @@ void setup_pager(void)
 
 	setenv("GIT_PAGER_IN_USE", "true", 1);
 
+retry:
 	/* spawn the pager */
 	prepare_pager_args(&pager_process, pager);
 	pager_process.in = -1;
 	strvec_push(&pager_process.env_array, "GIT_PAGER_IN_USE");
-	if (start_command(&pager_process))
-		return;
+	if (start_command(&pager_process)) {
+		/* chosen pager failed, try again with the default pager */
+		if (strcmp(pager, DEFAULT_PAGER)) {
+			free(pager);
+			pager = xstrdup(DEFAULT_PAGER);
+			goto retry;
+		} else {
+			/* default pager failed, let caller handle it */
+			free(pager);
+			return;
+		}
+	}
 
 	/* original process continues, but writes to the pipe */
 	dup2(pager_process.in, 1);
diff --git a/run-command.c b/run-command.c
index f329391154ae..d2b7647afdd8 100644
--- a/run-command.c
+++ b/run-command.c
@@ -19,6 +19,7 @@ void child_process_clear(struct child_process *child)
 {
 	strvec_clear(&child->args);
 	strvec_clear(&child->env_array);
+	memset(child, 0, sizeof(*child));
 }
 
 struct child_to_clean {
-- 
2.33.1