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

enh enh at google.com
Thu Apr 11 17:35:56 PDT 2019


On Thu, Apr 11, 2019 at 4:44 PM Rob Landley <rob at landley.net> wrote:
>
> On 4/11/19 12:18 PM, enh wrote:
> > Ping for direction here? I'm happy to implement whichever way you want to go.
> >
> > I think we had:
> >
> > 1. Don't implicitly flush in all the x* routines, add any missing explicit
> > flushes (of which I expect there will be very few).
> >
> > 2. Don't use the x* routines as much.
>
> I recently did:
>
> 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.

> The point of xprintf() and friends is if you have
>
>   yes | head
>
> then "yes" should end when stdout closes. It doesn't have to notice
> _imediately_, but generating potentially unbounded data into a pipe shouldn't
> keep going an arbitrarily long time, and we disable sigpipe because that's bugs
> waiting to happen.
>
> FILE * output flushes itself eventually, and then we'll notice the error. I
> think musl's FILE buffer is like 256 bytes and glibc was what, 4k? We don't need
> to flush that.

(glibc is 8KiB, fwiw, but i've never seen a libc use a bigger buffer than that.)

> The stdio code flushes when the output includes \n so xputc('\n') is already a
> flush inside libc anyway. I should grep or xprintf/xputc/xputs uses and see if
> there's an obvious one missing. (The password: prompt is the only one that comes
> to mind, toysh will need one too...)
>
> Oh, the other thing I have in this bucket is eliminating get_line(), which can't
> be done sanely without input blocking (and is largely why FILE * exists; if you
> can't push data back into a filehandle then you either read a byte at a time or
> remember the overshoot; get_line() reads a byte at a time and performance sucks
> and it makes strace output unreadable).
>
> I initially wrote it because posix-2001 didn't have getline() yet (it was a
> glibc extension I didn't want to rely on), but posix-2008 added it, so...

yeah, i've been able to clean up a *lot* of code thanks to getline().
(though anyone who thinks fgets() is evil really needs to look at its
truly evil twin fgetln()...)

> Rob
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-grep-add-line-buffered-and-fix-regular-buffering.patch
Type: text/x-patch
Size: 6449 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20190411/edc9ae54/attachment-0003.bin>


More information about the Toybox mailing list