On Saturday 04 June 2011 05:27 AM, Peter Samuelson wrote:
> Concept patch - not tested. Does this seem reasonable? I'd really
> prefer to do all our prompting on the terminal. The fact that this
> frees up stdin for things like 'svn patch' and 'svnrdump load' is only
> one reason.
>
I tested your patch in my Linux(ubuntu) box. It works with the following
tweaks.
> [[[
> For command-line svn clients, do interactive prompting using a terminal
> device, not stdin/stderr. This means "/dev/tty" on Unix, "CON:" on
> Windows. It is in fact what apr_password_get() already does.
>
> No effort is made to cache this open file. Printing prompts to the
> user's terminal should not be on a fast path anyway.
>
> * configure.ac
> (): Check for ctermid().
>
> * subversion/libsvn_subr
> (TOPLEVEL): Include<stdio.h> for fputs, ctermid. New macro TTY_FILE
> is platform-dependent.
>
> (open_tty, open_tty_apr): New functions. The latter is only needed
> because there's no way to convert between FILE and apr_file_t.
>
> (prompt): Various adjustments to call open_tty_apr() to get a
> filehandle, rather than apr_file_open_stdin and stderr.
>
> (maybe_print_realm, plaintext_prompt_helper): Call open_tty() instead
> of using stderr.
> ]]]
>
> Index: configure.ac
> ===================================================================
> --- configure.ac (revisione 1130333)
> +++ configure.ac (copia locale)
> @@ -768,7 +768,7 @@ dnl svn_error's default warning handler uses vfpri
> AC_FUNC_VPRINTF
>
> dnl check for functions needed in special file handling
> -AC_CHECK_FUNCS(symlink readlink)
> +AC_CHECK_FUNCS(symlink readlink ctermid)
>
>
> dnl Process some configuration options ----------
> Index: subversion/libsvn_subr/prompt.c
> ===================================================================
> --- subversion/libsvn_subr/prompt.c (revisione 1130333)
> +++ subversion/libsvn_subr/prompt.c (copia locale)
> @@ -29,6 +29,7 @@
>
> #include<apr_lib.h>
> #include<apr_poll.h>
> +#include<stdio.h>
>
> #include "svn_cmdline.h"
> #include "svn_string.h"
> @@ -41,6 +42,43 @@
>
>
>
> +#ifdef HAVE_CTERMID
> +#define TTY_FILE ctermid(NULL)
> +#else
> +#ifdef WIN32
> +#define TTY_FILE "CON:"
> +#else
> +#define TTY_FILE "/dev/tty"
> +#endif
> +#endif
> +
> +/* Retrieve a file pointer for console input/output.
> + * Failure typically means we do not have a controlling terminal. */
> +static svn_error_t *open_tty(apr_file_t **fp)
> +{
> + FILE *fp0 = fopen(TTY_FILE, "r+");
> + if (!fp0)
> + {
> + return svn_error_create(SVN_ERR_IO_WRITE_ERROR, NULL,
> + _("Can't open terminal"));
> + }
> + *fp = fp0;
> + return SVN_NO_ERROR;
> +}
> +
> +static svn_error_t *open_tty_apr(apr_file_t **fp,
> + apr_pool_t *pool)
> +{
> + apr_error_t status = apr_open_file(fp, TTY_FILE,
> + APR_FOPEN_READ | APR_FOPEN_WRITE,
> + APR_OS_DEFAULT, pool);
s/apr_error_t/apr_status_t
s/apr_open_file/apr_file_open
> + if (status)
> + {
> + return svn_error_wrap_apr(status, _("Can't open terminal"));
> + }
> + return SVN_NO_ERROR;
> +}
> +
> /* Wait for input on @a *f. Doing all allocations
> * in @a pool. This functions is based on apr_wait_for_io_or_timeout().
> * Note that this will return an EINTR on a signal.
> @@ -92,21 +130,24 @@ prompt(const char **result,
> * which were not included in svn_cmdline_prompt_baton_t,
> * we need to update svn_cmdline_prompt_user2 and its callers. */
> apr_status_t status;
> - apr_file_t *fp;
> char c;
> -
> + const char *prompt_stdout;
> svn_stringbuf_t *strbuf = svn_stringbuf_create("", pool);
>
> - status = apr_file_open_stdin(&fp, pool);
> - if (status)
> - return svn_error_wrap_apr(status, _("Can't open stdin"));
> + SVN_ERR(svn_cmdline_cstring_from_utf8(&prompt_stdout, prompt_msg, pool));
>
> if (! hide)
> {
> + apr_file_t *fp;
> svn_boolean_t saw_first_half_of_eol = FALSE;
> - SVN_ERR(svn_cmdline_fputs(prompt_msg, stderr, pool));
> - fflush(stderr);
>
> + SVN_ERR(open_tty_apr(&fp, pool));
> +
- SVN_ERR(open_tty_apr(&fp, pool));
+ status = open_tty_apr(&fp, pool);
> + apr_file_puts(prompt_stdout, fp);
> + if (status)
> + return svn_error_wrap_apr(status, _("Can't write to terminal"));
> + apr_file_flush(fp);
> +
> while (1)
> {
> /* Hack to allow us to not block for io on the prompt, so
> @@ -117,11 +158,11 @@ prompt(const char **result,
> if (APR_STATUS_IS_EINTR(status))
> continue;
> else if (status&& status != APR_ENOTIMPL)
> - return svn_error_wrap_apr(status, _("Can't read stdin"));
> + return svn_error_wrap_apr(status, _("Can't read from terminal"));
>
> status = apr_file_getc(&c, fp);
> if (status)
> - return svn_error_wrap_apr(status, _("Can't read stdin"));
> + return svn_error_wrap_apr(status, _("Can't read from terminal"));
>
> if (saw_first_half_of_eol)
> {
> @@ -149,13 +190,11 @@ prompt(const char **result,
>
> svn_stringbuf_appendbyte(strbuf, c);
> }
> + apr_file_close(fp);
> }
> else
> {
> - const char *prompt_stdout;
> size_t bufsize = 300;
> - SVN_ERR(svn_cmdline_cstring_from_utf8(&prompt_stdout, prompt_msg,
> - pool));
> svn_stringbuf_ensure(strbuf, bufsize);
>
> status = apr_password_get(prompt_stdout, strbuf->data,&bufsize);
> @@ -171,17 +210,24 @@ prompt(const char **result,
> /** Prompt functions for auth providers. **/
>
> /* Helper function for auth provider prompters: mention the
> - * authentication @a realm on stderr, in a manner appropriate for
> - * preceding a prompt; or if @a realm is null, then do nothing.
> + * authentication @a realm on the user's terminal, in a manner appropriate
> + * for preceding a prompt; or if @a realm is null, then do nothing.
> */
> static svn_error_t *
> maybe_print_realm(const char *realm, apr_pool_t *pool)
> {
> if (realm)
> {
> - SVN_ERR(svn_cmdline_fprintf(stderr, pool,
> - _("Authentication realm: %s\n"), realm));
> - fflush(stderr);
> + FILE *fp;
> +
> + SVN_ERR(open_tty(&fp));
> +
> + if (!fputs(apr_psprintf(pool, _("Authentication realm: %s\n"), realm),
> + fp))
> + {
> + return svn_error_create(SVN_ERR_IO_WRITE_ERROR, NULL, NULL);
> + }
> + fclose(fp);
> }
>
> return SVN_NO_ERROR;
> @@ -396,13 +442,16 @@ plaintext_prompt_helper(svn_boolean_t *may_save_pl
> svn_boolean_t answered = FALSE;
> svn_cmdline_prompt_baton2_t *pb = baton;
> const char *config_path = NULL;
> + FILE *fp;
>
> if (pb)
> SVN_ERR(svn_config_get_user_config_path(&config_path, pb->config_dir,
> SVN_CONFIG_CATEGORY_SERVERS, pool));
>
> - SVN_ERR(svn_cmdline_fprintf(stderr, pool, prompt_text, realmstring,
> + SVN_ERR(open_tty(&fp));
> + SVN_ERR(svn_cmdline_fprintf(fp, pool, prompt_text, realmstring,
> config_path));
> + fclose(fp);
>
> do
> {
But still I am getting few compilation warnings.
<snip>
../subversion/libsvn_subr/prompt.c: In function ‘open_tty’:
../subversion/libsvn_subr/prompt.c:65: warning: assignment from
incompatible pointer type
../subversion/libsvn_subr/prompt.c: In function ‘maybe_print_realm’:
../subversion/libsvn_subr/prompt.c:223: warning: passing argument 1 of
‘open_tty’ from incompatible pointer type
../subversion/libsvn_subr/prompt.c:57: note: expected ‘struct apr_file_t
**’ but argument is of type ‘struct FILE **’
../subversion/libsvn_subr/prompt.c: In function ‘plaintext_prompt_helper’:
../subversion/libsvn_subr/prompt.c:451: warning: passing argument 1 of
‘open_tty’ from incompatible pointer type
../subversion/libsvn_subr/prompt.c:57: note: expected ‘struct apr_file_t
**’ but argument is of type ‘struct FILE **’
../subversion/libsvn_subr/prompt.c:452: warning: format not a string
literal, argument types not checked
../subversion/libsvn_subr/prompt.c: In function ‘prompt’:
../subversion/libsvn_subr/prompt.c:180: warning: will never be executed
../subversion/libsvn_subr/prompt.c:188: warning: will never be executed
../subversion/libsvn_subr/prompt.c:147: warning: ‘status’ may be used
uninitialized in this function
</snip>
Now, I could run "svnrdump load" successfully in interactive mode. It
started to read from terminal instead of standard input stream.
Thanks & Regards,
Vijayaguru
Received on 2011-06-04 13:25:11 CEST