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

[Junio C Hamano <gitster@pobox.com>]

Tay Ray Chuan <rctay89@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 <rctay89@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 */