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".

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.

>  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 */
--
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

Reply via email to