From 31b49d9b653803e7c7fd18b21c8bdd86e3421668 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 3 Feb 2012 17:14:11 -0500 Subject: [PATCH 1/2] prompt: clean up strbuf usage The do_askpass function inherited a few bad habits from the original git_getpass. One, there's no need to strbuf_reset a buffer which was just initialized. And two, it's a good habit to use strbuf_detach to claim ownership of a buffer's string (even though in this case the owning buffer goes out of scope, so it's effectively the same thing). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- prompt.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/prompt.c b/prompt.c index 72ab9de2f94be3..64f817b36d9b65 100644 --- a/prompt.c +++ b/prompt.c @@ -21,7 +21,6 @@ static char *do_askpass(const char *cmd, const char *prompt) if (start_command(&pass)) exit(1); - strbuf_reset(&buffer); if (strbuf_read(&buffer, pass.out, 20) < 0) die("failed to get '%s' from %s\n", prompt, cmd); @@ -32,7 +31,7 @@ static char *do_askpass(const char *cmd, const char *prompt) strbuf_setlen(&buffer, strcspn(buffer.buf, "\r\n")); - return buffer.buf; + return strbuf_detach(&buffer, NULL); } char *git_prompt(const char *prompt, int flags) From 84d72733fc8cb90ac7fef04bb114da1e14d8b147 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 3 Feb 2012 17:16:02 -0500 Subject: [PATCH 2/2] prompt: fall back to terminal if askpass fails The current askpass code simply dies if calling an askpass helper fails. Worse, in some failure modes it doesn't even print an error (if start_command fails, then it prints its own error; if reading fails, we print an error; but if the command exits non-zero, finish_command fails and we print nothing!). Let's be more kind to the user by printing an error message when askpass doesn't work out, and then falling back to the terminal (which also may fail, of course, but we die already there with a nice message). While we're at it, let's clean up the existing error messages a bit. Now that our prompts are very long and contain quotes and colons themselves, our error messages are hard to read. So the new failure modes look like: [before, with a terminal] $ GIT_ASKPASS=false git push $ echo $? 128 [before, with no terminal, and we must give up] $ setsid git push fatal: could not read 'Password for 'https://peff@github.com': ': No such device or address [after, with a terminal] $ GIT_ASKPASS=false git push error: unable to read askpass response from 'false' Password for 'https://peff@github.com': [after, with no terminal, and we must give up] $ GIT_ASKPASS=false setsid git push error: unable to read askpass response from 'false' fatal: could not read Password for 'https://peff@github.com': No such device or address Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- prompt.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/prompt.c b/prompt.c index 64f817b36d9b65..d851807feb9849 100644 --- a/prompt.c +++ b/prompt.c @@ -9,6 +9,7 @@ static char *do_askpass(const char *cmd, const char *prompt) struct child_process pass; const char *args[3]; static struct strbuf buffer = STRBUF_INIT; + int err = 0; args[0] = cmd; args[1] = prompt; @@ -19,15 +20,21 @@ static char *do_askpass(const char *cmd, const char *prompt) pass.out = -1; if (start_command(&pass)) - exit(1); + return NULL; if (strbuf_read(&buffer, pass.out, 20) < 0) - die("failed to get '%s' from %s\n", prompt, cmd); + err = 1; close(pass.out); if (finish_command(&pass)) - exit(1); + err = 1; + + if (err) { + error("unable to read askpass response from '%s'", cmd); + strbuf_release(&buffer); + return NULL; + } strbuf_setlen(&buffer, strcspn(buffer.buf, "\r\n")); @@ -36,7 +43,7 @@ static char *do_askpass(const char *cmd, const char *prompt) char *git_prompt(const char *prompt, int flags) { - char *r; + char *r = NULL; if (flags & PROMPT_ASKPASS) { const char *askpass; @@ -47,12 +54,15 @@ char *git_prompt(const char *prompt, int flags) if (!askpass) askpass = getenv("SSH_ASKPASS"); if (askpass && *askpass) - return do_askpass(askpass, prompt); + r = do_askpass(askpass, prompt); } - r = git_terminal_prompt(prompt, flags & PROMPT_ECHO); if (!r) - die_errno("could not read '%s'", prompt); + r = git_terminal_prompt(prompt, flags & PROMPT_ECHO); + if (!r) { + /* prompts already contain ": " at the end */ + die("could not read %s%s", prompt, strerror(errno)); + } return r; }