[Toybox] [PATCH] watch: flush the buffer each round.

enh enh at google.com
Mon Mar 11 10:27:50 PDT 2024


On Fri, Mar 8, 2024 at 5:43 PM Rob Landley <rob at landley.net> wrote:
>
> I remember when the xprintf() family would do a flush and check for errors each
> write. That's why code like:
>
>         dprintf(1, "%c", pad<cmdlen ? '*' : ' ');
>         if (width) xputs(ss+(width>ctimelen ? 0 : width-1));
>         if (yy>=3) dprintf(1, "\r\n");
>
> Was allowable.
>
> I also remember when we had an xflush() that would catch errors if stdout
> barfed, instead of calling fflush() and ignoring the return code.

yeah, i didn't understand why you removed xflush(). but none of this
is relevant here --- the problem here is that we're not flushing _at
all_, so ToT watch is just broken.

> This code is mixing dprintf(1, ...) with printf() and fflush(stdout) in a way
> that seems unlikely to end well, and I would like to think it through when I'm
> not moving house. (Or have somebody whose brain isn't jello reassure me that
> it's coherent.)

not in the ToT copy it isn't. maybe you have local changes?

/tmp/toybox$ grep dprintf toys/other/watch.c
/tmp/toybox$

> Ideally, I would wean it entirely off of the conceptually broken FILE * output
> nonsense entirely to dprintf() and write(), because it's intentionally doing
> progressive output and trying to micromanage cache management there is unending
> pain. (We need a stdout with the nagle algorithm. Why did they only bother to do
> that for network sockets?)

aye, you keep saying, but that doesn't exist yet, and watch is broken today :-)

that said, my previous patch was suboptimal --- there are `continue`s
in the loop, so the flush makes more sense at the top. also i noticed
by inspection trying to work out what you meant about mixing stdio and
direct fd access (i don't use watch myself; this was a reported bug)
that -b was broken. so the new patch (attached) is a better fix. but
that fixes the normal case and -b for me...

> Rob
> _______________________________________________
> Toybox mailing list
> Toybox at lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-watch-flush-the-buffer-each-time-around-the-loop.patch
Type: application/octet-stream
Size: 1069 bytes
Desc: not available
URL: <http://lists.landley.net/pipermail/toybox-landley.net/attachments/20240311/f1ed028a/attachment.obj>


More information about the Toybox mailing list