[Toybox] [PATCH] grep: add --line-buffered and fix regular buffering.

[enh]

On Tue, Apr 16, 2019 at 3:21 PM Rob Landley <rob at landley.net> wrote:
>
> On 4/11/19 7:35 PM, enh wrote:
> > On Thu, Apr 11, 2019 at 4:44 PM Rob Landley <rob at landley.net> wrote:
> >> commit 8f882370be150d80969a1910c20b5d223d084b76
> >> Author: Rob Landley <rob at landley.net>
> >> Date:   Tue Apr 2 15:03:32 2019 -0500
> >>
> >>     Have xflush() only flush stdout (that's all it checks errors on),
> >>     and tweak a couple comments.
> >>
> >> but removing even that much flushing unless explicitly called is probably fine.
> >
> > cool... i've attached a new patch that applies cleanly on ToT.
>
> I meant removing it from libc, which I've just done.
>
> I can add the --line-buffered command line option, but I'm reluctant to add
> micromanagement knobs rather than have consistent sane behavior. I _think_ I
> want stdout to always be line buffered, and should just call that in main.c? (A
> command that really wants something different can set it again, but that seems
> like a fairly minor optimization? He says having implemented cat -u way back
> when, which ubuntu ignores now...)
>
> Line buffered is the behavior I've been assuming, but I see glibc defaults to
> "unbuffered"... in which case what is FILE * _for_ again? A strrchr() on each
> buffer with a flush to that point isn't that expensive, and most of the stuff
> that cares about micromanaging buffers is already using filehandles instead of
> FILE * anyway...

i think the problem here is that you're looking at this through the
wrong lens. i think your mental model is "interactive user", whereas
i'm thinking of them *and* of scripts/builds.

the traditional behavior of trusting the C library's buffering and
having just one exception (that i've ever seen) in grep _is_ sane.

i don't know where you misapprehensions about libc's behavior are
coming from... they're not true of bionic, nor any version of glibc
i've ever seen. POSIX explicitly says:

  When opened, the standard error stream is not fully buffered; the
standard input and standard output streams are fully buffered if and
only if the stream can be determined not to refer to an interactive
device.

that's the only behavior i've ever seen implemented, and that's the
behavior you want. scripts/builds don't get pessimized performance,
and interactive users don't have to wait for 1KiB/4KiB/8KiB.

(of course, this assumes that the application isn't flushing way too
often, which i claim is the real and only bug here.)

strictly speaking, glibc has a helper program (stdbuf(1)) that lets
you modify the buffering for the program it calls, but no other libc
(that i know of) has anything similar, and i've never seen the glibc
one used. as far as i know, the grep --line-buffered option, which is
in both BSD and GNU, is the only place where a command-line utility
explicitly offers fine control. and since Android's on-device grep has
always been [Net]BSD and the build grep has been GNU grep, both
device- and host-side expect this behavior, and control it where
necessary with --line-buffered.

there are definitely places where traditional tools do something
stupid, and no one actually wants the stupid, and i'm happy with those
deviations. but this is a place where i think toybox is behaving
differently to no advantage and some disadvantage, whereas stdio left
to its own devices (without constant hammering of the flush button)
does a good enough job for both distinct use cases.

> Rob