Re: getpass: use of termios.h problematic

[Simon Josefsson]

Paul Eggert <address@hidden> writes:

> Simon Josefsson <address@hidden> writes:
>
>> Ok to install?
>
> OK, with the following caveats:
>
> * Please put the standard implementation first, and the Windows implementation
>   second.  That'll make it easier to read.

Yup.

> * Please use standard GNU indenting style, e.g.,

I ran indent.

For reference, the patch below is what I installed.

Thanks!

Index: lib/ChangeLog
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/ChangeLog,v
retrieving revision 1.932
diff -u -p -r1.932 ChangeLog
--- lib/ChangeLog       25 Aug 2005 20:39:57 -0000      1.932
+++ lib/ChangeLog       25 Aug 2005 21:18:26 -0000
@@ -1,3 +1,9 @@
+2005-08-24  Simon Josefsson  <address@hidden>
+
+       * getpass.c: Add WIN32 implementation.  Conditionalize use of
+       termios.h, tcgetattr, tcsetattr and __fsetlocking.  Remove some
+       GLIBC specific code.
+
 2005-08-25  Paul Eggert  <address@hidden>
 
        Make regex safe for g++.  This fixes one real bug (an "err"
Index: lib/getpass.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/getpass.c,v
retrieving revision 1.11
diff -u -p -r1.11 getpass.c
--- lib/getpass.c       14 May 2005 06:03:58 -0000      1.11
+++ lib/getpass.c       25 Aug 2005 21:18:26 -0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 1992-2001, 2003, 2004 Free Software Foundation, Inc.
+/* Copyright (C) 1992-2001, 2003, 2004, 2005 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    This program is free software; you can redistribute it and/or modify
@@ -19,43 +19,28 @@
 # include <config.h>
 #endif
 
-#if !_LIBC
-# include "getpass.h"
-#endif
+#include "getpass.h"
 
-#if _LIBC
-# define HAVE_STDIO_EXT_H 1
-#endif
+#include <stdio.h>
+
+#if !defined WIN32
 
 #include <stdbool.h>
 
-#include <stdio.h>
 #if HAVE_STDIO_EXT_H
 # include <stdio_ext.h>
-#else
-# define __fsetlocking(stream, type) /* empty */
 #endif
-#if !_LIBC
-# include "getline.h"
+#if !HAVE___FSETLOCKING
+# define __fsetlocking(stream, type)   /* empty */
 #endif
 
-#include <termios.h>
-#include <unistd.h>
-
-#if _LIBC
-# include <wchar.h>
+#if HAVE_TERMIOS_H
+# include <termios.h>
 #endif
 
-#if _LIBC
-# define NOTCANCEL_MODE "c"
-#else
-# define NOTCANCEL_MODE
-#endif
+#include "getline.h"
 
-#if _LIBC
-# define flockfile(s) _IO_flockfile (s)
-# define funlockfile(s) _IO_funlockfile (s)
-#elif USE_UNLOCKED_IO
+#if USE_UNLOCKED_IO
 # include "unlocked-io.h"
 #else
 # if !HAVE_DECL_FFLUSH_UNLOCKED
@@ -80,18 +65,6 @@
 # endif
 #endif
 
-#if _LIBC
-# include <bits/libc-lock.h>
-#else
-# define __libc_cleanup_push(function, arg) /* empty */
-# define __libc_cleanup_pop(execute) /* empty */
-#endif
-
-#if !_LIBC
-# define __getline getline
-# define __tcgetattr tcgetattr
-#endif
-
 /* It is desirable to use this bit on systems that have it.
    The only bit of terminal state we want to twiddle is echoing, which is
    done in software; there is no need to change the state of the terminal
@@ -114,7 +87,7 @@ getpass (const char *prompt)
   FILE *tty;
   FILE *in, *out;
   struct termios s, t;
-  bool tty_changed;
+  bool tty_changed = false;
   static char *buf;
   static size_t bufsize;
   ssize_t nread;
@@ -122,7 +95,7 @@ getpass (const char *prompt)
   /* Try to write to and read from the terminal if we can.
      If we can't open the terminal, use stderr and stdin.  */
 
-  tty = fopen ("/dev/tty", "w+" NOTCANCEL_MODE);
+  tty = fopen ("/dev/tty", "w+");
   if (tty == NULL)
     {
       in = stdin;
@@ -136,39 +109,26 @@ getpass (const char *prompt)
       out = in = tty;
     }
 
-  /* Make sure the stream we opened is closed even if the thread is
-     canceled.  */
-  __libc_cleanup_push (call_fclose, tty);
-
   flockfile (out);
 
   /* Turn echoing off if it is on now.  */
-
-  if (__tcgetattr (fileno (in), &t) == 0)
+#if HAVE_TCGETATTR
+  if (tcgetattr (fileno (in), &t) == 0)
     {
       /* Save the old one. */
       s = t;
       /* Tricky, tricky. */
-      t.c_lflag &= ~(ECHO|ISIG);
-      tty_changed = (tcsetattr (fileno (in), TCSAFLUSH|TCSASOFT, &t) == 0);
+      t.c_lflag &= ~(ECHO | ISIG);
+      tty_changed = (tcsetattr (fileno (in), TCSAFLUSH | TCSASOFT, &t) == 0);
     }
-  else
-    tty_changed = false;
+#endif
 
   /* Write the prompt.  */
-#ifdef USE_IN_LIBIO
-  if (_IO_fwide (out, 0) > 0)
-    __fwprintf (out, L"%s", prompt);
-  else
-#endif
-    fputs_unlocked (prompt, out);
+  fputs_unlocked (prompt, out);
   fflush_unlocked (out);
 
   /* Read the password.  */
-  nread = __getline (&buf, &bufsize, in);
-
-#if !_LIBC
-  /* As far as is known, glibc doesn't need this no-op fseek.  */
+  nread = getline (&buf, &bufsize, in);
 
   /* According to the C standard, input may not be followed by output
      on the same stream without an intervening call to a file
@@ -180,7 +140,6 @@ getpass (const char *prompt)
      from POSIX version to POSIX version, so play it safe and invoke
      fseek even if in != out.  */
   fseek (out, 0, SEEK_CUR);
-#endif
 
   if (buf != NULL)
     {
@@ -193,25 +152,75 @@ getpass (const char *prompt)
          if (tty_changed)
            {
              /* Write the newline that was not echoed.  */
-#ifdef USE_IN_LIBIO
-             if (_IO_fwide (out, 0) > 0)
-               putwc_unlocked (L'\n', out);
-             else
-#endif
-               putc_unlocked ('\n', out);
+             putc_unlocked ('\n', out);
            }
        }
     }
 
   /* Restore the original setting.  */
+#if TCSETATTR
   if (tty_changed)
-    (void) tcsetattr (fileno (in), TCSAFLUSH|TCSASOFT, &s);
+    tcsetattr (fileno (in), TCSAFLUSH | TCSASOFT, &s);
+#endif
 
   funlockfile (out);
 
-  __libc_cleanup_pop (0);
-
   call_fclose (tty);
 
   return buf;
 }
+
+#else /* WIN32 */
+
+/* Windows implementation by Martin Lambers <address@hidden>,
+   improved by Simon Josefsson. */
+
+/* For PASS_MAX. */
+#include <limits.h>
+
+#ifndef PASS_MAX
+# define PASS_MAX 512
+#endif
+
+char *
+getpass (const char *prompt)
+{
+  char getpassbuf[PASS_MAX + 1];
+  size_t i = 0;
+  int c;
+
+  if (prompt)
+    {
+      fputs (prompt, stderr);
+      fflush (stderr);
+    }
+
+  for (;;)
+    {
+      c = _getch ();
+      if (c == '\r')
+       {
+         getpassbuf[i] = '\0';
+         break;
+       }
+      else if (i < PASS_MAX)
+       {
+         getpassbuf[i++] = c;
+       }
+
+      if (i >= PASS_MAX)
+       {
+         getpassbuf[i] = '\0';
+         break;
+       }
+    }
+
+  if (prompt)
+    {
+      fputs ("\r\n", stderr);
+      fflush (stderr);
+    }
+
+  return strdup (getpassbuf);
+}
+#endif
Index: m4/ChangeLog
===================================================================
RCS file: /cvsroot/gnulib/gnulib/m4/ChangeLog,v
retrieving revision 1.701
diff -u -p -r1.701 ChangeLog
--- m4/ChangeLog        25 Aug 2005 20:57:11 -0000      1.701
+++ m4/ChangeLog        25 Aug 2005 21:18:26 -0000
@@ -1,3 +1,8 @@
+2005-08-24  Simon Josefsson  <address@hidden>
+
+       * getpass.m4: Check for termios.h, tcgetattr, tcsetattr, and
+       __fsetlocking.
+
 2005-08-25  Stepan Kasal  <address@hidden>
 
        * regex.m4: Add AC_PREREQ(2.50).
Index: m4/getpass.m4
===================================================================
RCS file: /cvsroot/gnulib/gnulib/m4/getpass.m4,v
retrieving revision 1.10
diff -u -p -r1.10 getpass.m4
--- m4/getpass.m4       16 Apr 2005 18:49:09 -0000      1.10
+++ m4/getpass.m4       25 Aug 2005 21:18:26 -0000
@@ -35,7 +35,7 @@ AC_DEFUN([gl_FUNC_GETPASS_GNU],
 
 # Prerequisites of lib/getpass.c.
 AC_DEFUN([gl_PREREQ_GETPASS], [
-  AC_CHECK_HEADERS_ONCE(stdio_ext.h)
+  AC_CHECK_HEADERS_ONCE(stdio_ext.h termios.h)
+  AC_CHECK_FUNCS_ONCE(__fsetlocking tcgetattr tcsetattr)
   AC_CHECK_DECLS_ONCE([fflush_unlocked flockfile fputs_unlocked funlockfile 
putc_unlocked])
-  :
 ])