subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Samuelson <pe...@p12n.org>
Subject [RFC] [PATCH] Use console, not stdin/stderr, for prompting
Date Fri, 03 Jun 2011 23:57:54 GMT

[Peter Samuelson]
> Hrrrmph.  I think libsvn_subr/prompt.c could use a large dose of
> "/dev/tty".  Right now it mostly uses stderr and stdin, but for
> password prompts it uses /dev/tty on modern Unix ... indirectly,
> by calling apr_password_get() which calls getpass(), an OS library
> function.

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.

Peter

[[[
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);
+  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));
+
+      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
     {

Mime
View raw message