[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: yesno module consumes too much input

From: Eric Blake-1
Subject: Re: yesno module consumes too much input
Date: Fri, 17 Aug 2007 16:11:18 -0700 (PDT)

> >     Don't consume too much input when stdin is seekable.
> >     * lib/yesno.c (yesno): Flush seekable stdin.
> >     * modules/yesno (Depends-on): Add fflush.
> Won't this patch result in lots of unnecessary system calls for a
> program that reads a lot of 'y's and 'n'n from a file?

Ouch - yes, it can result in way more system calls than strictly
necessary; there is no need to flush stdin when more data
may be read.  An atexit approach is definitely more efficient.

> > The closein module is capable of resolving this problem.
> > However, rather than fix all of the affected applications
> > one-by-one (the list includes, but is not limited to,
> > coreutils' cp, ln, mv, rm, and remove; and findutils' find),
> > it would be nicer to fix this just once in yesno.
> (There is no 'remove' command.)

(oops - I saw yesno in remove.c; which is shared by rm,
rmdir, and mv, and typed without thinking).

> I see another problem with the proposed approach, other than the
> performance issue.  If a program like 'mv' has read from standard
> input, but then the fseeko fails at the end, then 'mv' should report
> an error to the user.
> My kneejerk reaction is that a modified 'closein' is a better way to
> go here.  That is, the affected programs (and there are not that many
> of them) should invoke close_stdin if they have attempted to read
> anything from stdin and have not read end-of-file.

OK, you are pretty convincing.  It still seems like most clients
of yesno only read stdin on condition of interactive responses,
therefore, 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).

I'll propose an alternate patch, along with the gnulib testsuite
addition I have been fiddling with, later on.  The benefits of
having yesno install the atexit handler is that it is still
minimally invasive to all the yesno clients (I always like patching
root cause, rather than repeating the same patch to all the
clients of a buggy API).

Eric Blake

View this message in context: 
Sent from the Gnulib mailing list archive at Nabble.com.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]