Re: yesno module consumes too much input

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Paul Eggert on 8/17/2007 5:43 PM:
> Eric Blake-1 <address@hidden> writes:
> 
>> is it reasonable to have yesno install an atexit
>> handler on first invocation?  If the handler is not present,
>> then stdin was never used (at least not by yesno).
> 
> Yes, that sounds reasonable to me.

How about the following patch?  Tested on Linux, including that test-yesno
detected failure prior to the yesno.c patch.

> 
> Also, I forgot, you also mentioned something about the fact that
> rpmatch.c should use nl_langinfo instead of gettext.  I agree with
> that.  I think rpmatch shouldn't mess with gettext; if the current
> system lacks nl_langinfo it should fall back on the !ENABLE_NLS code.

It looks like Bruno is tackling the rpmatch issue.

2007-08-18  Eric Blake  <address@hidden>

        Ensure yesno does not consume too much seekable stdin.
        * modules/yesno (Depends-on): Add closein.
        * lib/closein.c (close_stdin_only): New function.
        (close_stdin): Make closing stdout optional.
        * lib/closein.h (close_stdin_only): Add declaration.
        * lib/yesno.c (yesno): Register to call close_stdin at exit.
        * modules/yesno-tests (Files): New module.
        * tests/test-yesno.c (main): New file.
        * tests/test-yesno.sh: Likewise.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGxx+G84KuGfSFAYARAn79AJ0S/ECu8ft/mxqXTb0CWqDAdVaKxACfWM+x
L2BRQrzeY+hIy68NXkTmt8c=
=VRuG
-----END PGP SIGNATURE-----

Index: lib/closein.c
===================================================================
RCS file: /sources/gnulib/gnulib/lib/closein.c,v
retrieving revision 1.3
diff -u -p -r1.3 closein.c
--- lib/closein.c       27 Apr 2007 17:14:40 -0000      1.3
+++ lib/closein.c       18 Aug 2007 16:27:30 -0000
@@ -36,6 +36,8 @@
 
 static const char *file_name;
 
+static bool closein_only;
+
 /* Set the file name to be reported in the event an error is detected
    on stdin by close_stdin.  See also close_stdout_set_file_name, if
    an error is detected when closing stdout.  */
@@ -45,9 +47,20 @@ close_stdin_set_file_name (const char *f
   file_name = file;
 }
 
+/* Change whether close_stdin will also call close_stdout, and return
+   the previous state of the flag.  */
+bool
+close_stdin_only (bool value)
+{
+  bool old = closein_only;
+  closein_only = value;
+  return old;
+}
+
 /* Close standard input, rewinding any unused input if stdin is
    seekable.  On error, issue a diagnostic and _exit with status
-   'exit_failure'.  Then call close_stdout.
+   'exit_failure'.  Then call close_stdout, unless close_stdin_only
+   has been called.
 
    Most programs can get by with close_stdout.  close_stdin is only
    needed when a program wants to guarantee that partially read input
@@ -98,7 +111,8 @@ close_stdin (void)
        error (0, errno, "%s", close_error);
     }
 
-  close_stdout ();
+  if (!closein_only)
+    close_stdout ();
 
   if (fail)
     _exit (exit_failure);
Index: lib/closein.h
===================================================================
RCS file: /sources/gnulib/gnulib/lib/closein.h,v
retrieving revision 1.1
diff -u -p -r1.1 closein.h
--- lib/closein.h       12 Apr 2007 16:11:40 -0000      1.1
+++ lib/closein.h       18 Aug 2007 16:27:30 -0000
@@ -19,11 +19,14 @@
 #ifndef _GL_CLOSEIN_H
 # define _GL_CLOSEIN_H 1
 
+#include <stdbool.h>
+
 # ifdef __cplusplus
 extern "C" {
 # endif
 
 void close_stdin_set_file_name (const char *file);
+bool close_stdin_only (bool);
 void close_stdin (void);
 
 # ifdef __cplusplus
Index: lib/yesno.c
===================================================================
RCS file: /sources/gnulib/gnulib/lib/yesno.c,v
retrieving revision 1.17
diff -u -p -r1.17 yesno.c
--- lib/yesno.c 14 Dec 2006 18:47:36 -0000      1.17
+++ lib/yesno.c 18 Aug 2007 16:27:30 -0000
@@ -1,6 +1,6 @@
 /* yesno.c -- read a yes/no response from stdin
 
-   Copyright (C) 1990, 1998, 2001, 2003, 2004, 2005, 2006 Free
+   Copyright (C) 1990, 1998, 2001, 2003, 2004, 2005, 2006, 2007 Free
    Software Foundation, Inc.
 
    This program is free software; you can redistribute it and/or modify
@@ -24,14 +24,17 @@
 #include <stdlib.h>
 #include <stdio.h>
 
+#include "closein.h"
+
 #if ENABLE_NLS
 # include "getline.h"
 #endif
 
-/* Return true if we read an affirmative line from standard input.  */
 
 extern int rpmatch (char const *response);
 
+/* Return true if we read an affirmative line from standard input.  */
+
 bool
 yesno (void)
 {
@@ -60,5 +63,18 @@ yesno (void)
     c = getchar ();
 #endif
 
+  /* Assume that yesno is the only client of stdin for a given
+     program.  Thus, if we get here, we need to clean up stdin on
+     exit.  */
+  {
+    static bool init;
+    if (!init)
+      {
+       init = true;
+       close_stdin_only (true);
+       atexit (close_stdin);
+      }
+  }
+
   return yes;
 }
Index: modules/yesno
===================================================================
RCS file: /sources/gnulib/gnulib/modules/yesno,v
retrieving revision 1.13
diff -u -p -r1.13 yesno
--- modules/yesno       13 Oct 2006 12:40:23 -0000      1.13
+++ modules/yesno       18 Aug 2007 16:27:30 -0000
@@ -8,6 +8,7 @@ lib/yesno.h
 m4/yesno.m4
 
 Depends-on:
+closein
 getline
 rpmatch
 stdbool
Index: modules/yesno-tests
===================================================================
RCS file: modules/yesno-tests
diff -N modules/yesno-tests
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ modules/yesno-tests 18 Aug 2007 16:27:30 -0000
@@ -0,0 +1,14 @@
+Files:
+tests/test-yesno.c
+tests/test-yesno.sh
+
+Depends-on:
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-yesno.sh
+TESTS_ENVIRONMENT += EXEEXT='@EXEEXT@'
+check_PROGRAMS += test-yesno
+EXTRA_DIST += test-yesno.sh
+test_yesno_LDADD = $(LDADD) @LIBINTL@
Index: tests/test-yesno.c
===================================================================
RCS file: tests/test-yesno.c
diff -N tests/test-yesno.c
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/test-yesno.c  18 Aug 2007 16:27:30 -0000
@@ -0,0 +1,37 @@
+/* Test of yesno module.
+   Copyright (C) 2007 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "yesno.h"
+
+char *program_name;
+
+int
+main (int argc, char **argv)
+{
+  int i = 1;
+  program_name = argv[0];
+  if (1 < argc)
+    i = atoi (argv[1]);
+  while (i--)
+    puts (yesno () ? "Y" : "N");
+  return 0;
+}
Index: tests/test-yesno.sh
===================================================================
RCS file: tests/test-yesno.sh
diff -N tests/test-yesno.sh
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ tests/test-yesno.sh 18 Aug 2007 16:27:30 -0000
@@ -0,0 +1,48 @@
+#!/bin/sh
+
+tmpfiles=
+trap 'rm -fr $tmpfiles' 1 2 3 15
+
+p=t-yesno-
+tmpfiles="${p}in.tmp ${p}xout.tmp ${p}out.tmp"
+
+# Test with seekable stdin; followon process must see remaining data
+cat <<EOF > ${p}in.tmp
+nnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn
+yn
+y
+n
+EOF
+cat <<EOF > ${p}xout.tmp
+N
+Y
+Y
+n
+EOF
+(./test-yesno${EXEEXT}; ./test-yesno${EXEEXT} 2; cat) \
+  < ${p}in.tmp > ${p}out.tmp || exit 1
+cmp ${p}xout.tmp ${p}out.tmp || exit 1
+
+(./test-yesno${EXEEXT} 2; ./test-yesno${EXEEXT}; cat) \
+  < ${p}in.tmp > ${p}out.tmp || exit 1
+cmp ${p}xout.tmp ${p}out.tmp || exit 1
+
+# Test for lack of error on pipe
+cat <<EOF > ${p}xout.tmp
+Y
+N
+EOF
+echo yes | ./test-yesno${EXEEXT} 2 > ${p}out.tmp || exit 1
+cmp ${p}xout.tmp ${p}out.tmp || exit 1
+
+# Test for lack of error when nothing is read
+cat <<EOF > ${p}xout.tmp
+N
+EOF
+./test-yesno${EXEEXT} </dev/null > ${p}out.tmp || exit 1
+cmp ${p}xout.tmp ${p}out.tmp || exit 1
+
+# Cleanup
+rm -fr $tmpfiles
+
+exit 0