Re: [PATCH v3 1/2] add interface for /dev/tty interaction

[Jeff King]

On Sun, Aug 05, 2012 at 01:11:47PM -0700, Junio C Hamano wrote:

 Tay Ray Chuan rcta...@gmail.com writes:
 
  Factor out the opening and closing of /dev/tty from
  git_terminal_prompt(), so that callers may first test if a controlling
  terminal is available before proceeding with prompting proper.
 
  When HAVE_DEV_TTY is not defined, terminal_open() falls back to checking
  tty-ness of stdin and stderr, as getpass() uses them both.
 
  Signed-off-by: Tay Ray Chuan rcta...@gmail.com
  ---
 
 This is not your fault but seeing term_t made me go eek, yuck.

Agreed.

 As far as I can see, use of FILE * in existing compat/terminal.c
 is not buying us anything useful.  The stdio calls made on FILE *fh
 are only fopen(), fputs(), fflush() and fclose(), and everything
 else goes through fileno(fh).
 
 So perhaps it is a saner approach to fix that function first before
 this patch so that it works on file descriptors.

Yeah, I think that is a good path. I think my original use of stdio
was mostly because I started by paring down glibc's implementation of
getpass.  Since we have niceties like write_in_full, I don't think
there's any reason not to just skip stdio.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/2] add interface for /dev/tty interaction

[Tay Ray Chuan]

Factor out the opening and closing of /dev/tty from
git_terminal_prompt(), so that callers may first test if a controlling
terminal is available before proceeding with prompting proper.

When HAVE_DEV_TTY is not defined, terminal_open() falls back to checking
tty-ness of stdin and stderr, as getpass() uses them both.

Signed-off-by: Tay Ray Chuan rcta...@gmail.com
---
 compat/terminal.c | 52 
 compat/terminal.h | 10 ++
 2 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/compat/terminal.c b/compat/terminal.c
index 6d16c8f..c85d5c7 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -24,15 +24,21 @@ static void restore_term_on_signal(int sig)
raise(sig);
 }
 
-char *git_terminal_prompt(const char *prompt, int echo)
+term_t terminal_open(void)
+{
+   return fopen(/dev/tty, w+);
+}
+
+int terminal_close(term_t term)
+{
+   return fclose(term);
+}
+
+char *terminal_prompt(term_t term, const char *prompt, int echo)
 {
static struct strbuf buf = STRBUF_INIT;
int r;
-   FILE *fh;
-
-   fh = fopen(/dev/tty, w+);
-   if (!fh)
-   return NULL;
+   FILE *fh = term;
 
if (!echo) {
struct termios t;
@@ -64,18 +70,48 @@ char *git_terminal_prompt(const char *prompt, int echo)
}
 
restore_term();
-   fclose(fh);
 
if (r == EOF)
return NULL;
return buf.buf;
 }
 
+char *git_terminal_prompt(const char *prompt, int echo)
+{
+   char *ret;
+   term_t term;
+
+   term = terminal_open();
+   if (!term)
+   return NULL;
+
+   ret = terminal_prompt(term, prompt, echo);
+
+   terminal_close(term);
+
+   return ret;
+}
+
 #else
 
-char *git_terminal_prompt(const char *prompt, int echo)
+term_t terminal_open()
+{
+   return isatty(0)  isatty(2);
+}
+
+int terminal_close(term_t term)
+{
+   return 0;
+}
+
+char *terminal_prompt(term_t term, const char *prompt, int echo)
 {
return getpass(prompt);
 }
 
+char *git_terminal_prompt(const char *prompt, int echo)
+{
+   return terminal_prompt(prompt, echo);
+}
+
 #endif
diff --git a/compat/terminal.h b/compat/terminal.h
index 97db7cd..cf2aa10 100644
--- a/compat/terminal.h
+++ b/compat/terminal.h
@@ -1,6 +1,16 @@
 #ifndef COMPAT_TERMINAL_H
 #define COMPAT_TERMINAL_H
 
+#ifdef HAVE_DEV_TTY
+typedef FILE *term_t;
+#else
+typedef int term_t;
+#endif
+
+term_t terminal_open();
+int terminal_close(term_t term);
+char *terminal_prompt(term_t term, const char *prompt, int echo);
+
 char *git_terminal_prompt(const char *prompt, int echo);
 
 #endif /* COMPAT_TERMINAL_H */
-- 
1.7.12.rc1.187.g6dd9156

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html