Re: yesno module consumes too much input

[Eric Blake]

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

According to Bruno Haible on 8/18/2007 11:53 AM:
> Paul Eggert replied to Eric Blake:
>>> 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.
> 
> Still, I haven't understood what is so special about the 'yesno' function.
> The code in close_stdin() is needed if _any_ input has been done on stdin,
> no?

Yes, in general, an application should close stdin if it used it, and
should exit with non-zero status if there was a read error that affected
operation.  And if you don't care about read errors, POSIX (but not glibc)
already takes care of the automatic close of stdin, including the
repositioning to unread data on seekable files as part of exit().

However, the specific problem with yesno() is that the API does not make
it obvious whether we used stdin, or whether we bypassed streams
altogether by using read(STDIN_FILENO).  Since yesno abstracts away how
the input was obtained, it should also abstract away if and how the input
stream is synchronized at program exit.  This is different from readline,
getline, and other functions that make it obvious that stdin (or some
other stream) is being used.

You are correct that getpass would be another place to look at whether
stdin is flushed correctly, but as getpass prefers to use /dev/tty, and is
not called repetitively (unlike yesno), perhaps it is simpler to just
patch getpass.c to call fflush(stdin) when /dev/tty was not available and
stdin is seekable, rather than worrying about close_stdin.  (Not to
mention that getpass is seldom called on seekable input, because people
seldom store passwords in plain-text seekable files).

> 
> In that case, rather than modifying 'yesno', 'readline', 'getline', 'getpass'
> etc., wouldn't it be more reliable to call close_stdin() always,

I was trying to patch multiple programs at once (mv, rm, ln, find, ...) by
fixing yesno, rather than writing individual patches for each of those
programs.  But yes, we could go the other way and add atexit(close_stdin)
to all programs that use yesno(), and leave yesno() alone.

> and optimize
> close_stdin() to do nothing if stdin is still in the initial state (i.e.
> has no in-memory buffer). This can be done through in-memory tests, such as
> essentially (cf. lib/fseeko.c):

m4 already calls close_stdin always, whether or not it read from stdin, so
the current implementation is correct even when stdin is still in the
initial state.  However, if you want to optimize it further, such patches
are worth considering.

- --
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

iD8DBQFGxzfy84KuGfSFAYARAqHGAJ4k7KdK9yrxTYazrG+WLd0+jfF+HgCaA1ZH
JKmXkBpxzrKl0n/NSjYdu1E=
=RNv3
-----END PGP SIGNATURE-----