-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
According to Bruno Haible on 8/18/2007 11:38 AM:
> Hi Eric,
>
>> How about the following patch?
>> * lib/closein.c (close_stdin_only): New function.
>> (close_stdin): Make closing stdout optional.
>> * lib/closein.h (close_stdin_only): Add declaration.
>
> This API of closein.h can easily lead to bugs
Good catch.
>
> To make this API more easy to understand, how about this?
> 1) Rename the function 'close_stdin' to 'close_stdin_stdout',
> 2) Add a function 'close_stdin', which closes stdin only. In the
> implementation, arrange that when 'close_stdin' and 'close_stdin_stdout'
> are both called, the stdin part is done only once.
> 3) Don't add a 'close_stdin_only' setter.
As in the following?
Note that I still have a question on behavior. There is no way to query
what has previously been registered via atexit, so in this sequence:
atexit(close_stdout);
atexit(close_stdin);
results in calling close_stdin first. From there, if close_stdin detects
an error, there is the potential for stdout/stderr to be unflushed (and
thus output data lost) when close_stdin calls _exit. Whereas:
atexit(close_stdin_stdout);
flushes stdout before calling _exit.
About all I could think of that would avoid this problem is by adding a
new API, such as close_*_onexit(), used in place of the current
atexit(close_*), such that closein and closeout could communicate via
global variables which of the various close_* functions have been
registered, and thus do all the cleanup and flushing via a single atexit
handler. But that is more invasive, as there are already a number of
programs using atexit(close_stdout).
2007-08-18 Eric Blake <[EMAIL PROTECTED]>
Ensure yesno does not consume too much seekable stdin.
* modules/yesno (Depends-on): Add closein.
* lib/closein.c (close_stdin_stdout): New function.
(close_stdin): No longer close stdout.
* lib/closein.h (close_stdin_stdout): Add declaration.
* NEWS: Document API change.
* 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 [EMAIL PROTECTED]
-----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
iD8DBQFGx1Kc84KuGfSFAYARAufOAJ40pm1AMqb2XdIc25sBulewkrQHLACgyaGT
KkW5ISIfL1qitoRP1ndiK0k=
=DfIc
-----END PGP SIGNATURE-----
Index: NEWS
===================================================================
RCS file: /sources/gnulib/gnulib/NEWS,v
retrieving revision 1.29
diff -u -p -r1.29 NEWS
--- NEWS 18 Aug 2007 07:16:53 -0000 1.29
+++ NEWS 18 Aug 2007 18:55:44 -0000
@@ -6,6 +6,9 @@ User visible incompatible changes
Date Modules Changes
+2007-08-18 closein close_stdin now leaves stdout alone. Use the
+ new close_stdin_stdout for the former behavior.
+
2007-08-18 idcache Now provides prototypes in "idcache.h".
2007-08-10 xstrtol The STRTOL_FATAL_ERROR macro is removed.
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 18:55:44 -0000
@@ -45,9 +45,42 @@ close_stdin_set_file_name (const char *f
file_name = file;
}
+
+/* Implementation for both close_stdin and close_stdin_stdout;
+ close_stdout is called if FLAG is set. */
+static void
+close_stdin_common (bool flag)
+{
+ bool fail = false;
+
+ /* Only attempt flush if stdin is seekable, as fflush is entitled to
+ fail on non-seekable streams. */
+ if (fseeko (stdin, 0, SEEK_CUR) == 0 && fflush (stdin) != 0)
+ fail = true;
+ if (close_stream (stdin) != 0)
+ fail = true;
+ if (fail)
+ {
+ /* Report failure, but defer exit until after closing stdout,
+ since the failure report should still be flushed. */
+ char const *close_error = _("error closing file");
+ if (file_name)
+ error (0, errno, "%s: %s", quotearg_colon (file_name),
+ close_error);
+ else
+ error (0, errno, "%s", close_error);
+ }
+
+ if (flag)
+ close_stdout ();
+
+ if (fail)
+ _exit (exit_failure);
+}
+
/* 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'.
Most programs can get by with close_stdout. close_stdin is only
needed when a program wants to guarantee that partially read input
@@ -78,28 +111,13 @@ close_stdin_set_file_name (const char *f
void
close_stdin (void)
{
- bool fail = false;
-
- /* Only attempt flush if stdin is seekable, as fflush is entitled to
- fail on non-seekable streams. */
- if (fseeko (stdin, 0, SEEK_CUR) == 0 && fflush (stdin) != 0)
- fail = true;
- if (close_stream (stdin) != 0)
- fail = true;
- if (fail)
- {
- /* Report failure, but defer exit until after closing stdout,
- since the failure report should still be flushed. */
- char const *close_error = _("error closing file");
- if (file_name)
- error (0, errno, "%s: %s", quotearg_colon (file_name),
- close_error);
- else
- error (0, errno, "%s", close_error);
- }
+ close_stdin_common (false);
+}
- close_stdout ();
+/* Like close_stdin, except also call close_stdout. */
- if (fail)
- _exit (exit_failure);
+void
+close_stdin_stdout (void)
+{
+ close_stdin_common (true);
}
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 18:55:44 -0000
@@ -25,6 +25,7 @@ extern "C" {
void close_stdin_set_file_name (const char *file);
void close_stdin (void);
+void close_stdin_stdout (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 18:55:44 -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,17 @@ 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;
+ 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 18:55:44 -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 18:55:44 -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 18:55:44 -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 18:55:44 -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
