<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Feb 25, 2019 at 8:46 AM Rob Landley <<a href="mailto:rob@landley.net">rob@landley.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 2/24/19 10:56 PM, enh wrote:<br>
> <br>
> <br>
> On Sun, Feb 24, 2019, 15:32 Rob Landley <<a href="mailto:rob@landley.net" target="_blank">rob@landley.net</a><br>
> <mailto:<a href="mailto:rob@landley.net" target="_blank">rob@landley.net</a>>> wrote:<br>
> <br>
> On 2/24/19 4:27 PM, enh via Toybox wrote:<br>
> > This is slightly more involved than just calling setlinebuf(3) because<br>
> > the code was already implicitly flushing more than once per line thanks<br>
> > to all the calls to the 'x' wrappers around stdio.<br>
> <br>
> Most of them can go, I tend to err on the side of error checking (which requires<br>
> a flush to see if it worked) and then pull back later. (The xexit() code checks<br>
> at the end, but you want to catch things like "disk full" or a broken pipe<br>
> earlier rather than sitting there spinning...)<br>
> <br>
> <br>
> I think all of them should go. That would match GNU and BSD. Remember that<br>
> you'll get your disk full or broken pipe every time you fill the stdio buffer<br>
> *or* automatically after each line if you're outputting to a terminal anyway.<br>
> (And in the uncommon case you really do care, that's exactly what<br>
> --line-buffered is for.)<br>
> <br>
> Optimizing for the exceptional case where there are large gaps in the output<br>
> *and* you don't want all the output anyway, at the expense of performance in<br>
> successful cases seems wrong. <br>
<br>
Hmmm... You have a point.<br>
<br>
> To me this is an even worse idea than the current x* behavior. At least I've<br>
> learned that all the x* output functions cause an unnecessary flush because<br>
> we've fixed this over-zealous flushing in several places already. (And those are<br>
> just the ones we've caught.)<br>
<br>
I've been moving xprintf() to printf() in several places myself.<br>
<br>
I don't want a toybox command to produce endless output without checking for<br>
error ("sed blah bigfile.img | head" shouldn't run a long time, for example).<br></blockquote><div><br></div><div>true, but sed is one of those cases where there is regular _explicit_ flushing and GNU and BSD seem to agree.</div><div><br></div><div>what i want to avoid is that if we do 's/a/A/g' on `abababababababab\n` we flush nine times rather than one :-)</div><div><br></div><div>since i'm trying to push us down this "flushes should be explicit" path, i'm happy to be the one on the spot for fixing any missing places :-)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
And debian keeps breaking "tee" so:<br>
<br>
(while true; do echo -n .; sleep 1; done) | tee /dev/null<br>
<br>
Won't produce output for 4k and then suddenly produces multiple screens full.<br>
(They fix it, they break it, they fix it again...)<br>
<br>
> I think part of the reason we've had trouble with this is because it's implicit.<br>
> Adding *another* piece of magic seems like step in the wrong direction,<br>
> especially for something (early flushing) that seems like it's usually a mistake<br>
> anyway.<br>
<br>
I think what I usually want to do is check for errors but not flush. (There<br>
isn't a standard write() buffer size, an officially policy for _when_ it gets<br>
flushed out, and nothing like a timeout, which is why I was flushing it in the<br>
first place, but that turned out to be worse...)<br>
<br>
> (This specific suggestion seems doubly dangerous because it *breaks* xflush ---<br>
> the one *explicit* way to cause a flush. I'm all in favor of xflush. It's how we<br>
> fixed things like top to reduce flicker, for example.)<br>
<br>
To be honest I was calling xflush() from so many places because it was an easy<br>
way to get the if (ferror) perror_exit("write"); Possibly what I want is an<br>
xferror() that xflush() can call...<br></blockquote><div><br></div><div>where would you want to call xferror that xflush wouldn't be appropriate? (fread is the main place one needs ferror normally, and toybox has remarkably few calls to fread, neither of which seem to actually need to care.)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> Personally I'm not 100% convinced that putsn and putsnl pull their weight at<br>
> all...<br>
<br>
Neither am I, but it's annoying there _isn't_ one in the standard library. (And<br>
puts() adding a newline but fputs() _not_ doing so is just irritating.)<br></blockquote><div><br></div><div>that and the fact that they couldn't decide whether the FILE* came first (in most cases) or last (in a couple of annoying exceptions) annoy me far more than they should. but they mean that even 30 years after meeting them, i still need to check the man page to use them. (or, more usually, just type whatever and let the compiler correct me.)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
That said, you can fwrite() a gigabyte piped through cat and it all goes out in<br>
one gulp even with glibc. (Yes, I wrote a test program.) This is why I have a<br>
writeall() for filehandles but not an fwriteall(): because I _think_ libc can be<br>
trusted to retry short writes? (Something's doing it. When I worry about data<br>
integrity I do it myself with filehandles and writeall() anyway, although<br>
get_line() in lib.c is a pre-posix-2008 relic I need to clean out. It's on the<br>
todo list...)<br>
<br>
And trusting fwrite() is only for squishy values of "trust" because posix won't<br>
come out and SAY it retries short writes. In fact posix says that being<br>
interrupted by a signal makes it return a short write which I believe is _not_<br>
what Linux does with things like SIGSTOP/CONT and SIGCHLD, those the OS will<br>
retry internally, and yes I know this because I broke it:<br>
<br>
<a href="http://lkml.iu.edu/hypermail/linux/kernel/0503.3/1756.html" rel="noreferrer" target="_blank">http://lkml.iu.edu/hypermail/linux/kernel/0503.3/1756.html</a><br>
<br>
And then the implementation changed (splice went in circa<br>
<a href="https://lwn.net/Articles/178199/" rel="noreferrer" target="_blank">https://lwn.net/Articles/178199/</a> and the pipe plumbing got a complete rewrite)<br>
which had subtle behavior diferences... *shrug* The usual.<br>
<br>
(If you're wondering why I get a bit paranoid about this stuff in places, it's<br>
scar tissue and old trauma. I have another todo item to investigate the<br>
sigaction(SA_RESTART) behavior's impact on this stuff and what the defaults can<br>
be trusted to _be_ these days, but again: todo list runneth over and this ain't<br>
my $DAYJOB...)<br></blockquote><div><br></div><div>i've gone a step further and admitted default. do what you like, the flash is lying to the kernel anyway, and it's all down to trust in the end. do you really believe that the blocks are at least in some kind of non-volatile storage and won't get lost between where they are now and where they're supposed to end up? i think the best answer is only "mostly".</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> I'd happily use printf("%s", s) for the former given that neither reader<br>
> nor writer needs to think about what that means, and it composes better with any<br>
> neighboring printf or putchar or whatever.<br>
<br>
Except too many times it's _not_ composed...<br>
<br>
$ grep '("%s",' toys/*/*.c | wc<br>
52 252 3250<br></blockquote><div><br></div><div>aye, but that's fixable.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> Likewise "%*.*s", which is widely<br>
> used and a generally useful thing for the reader to learn. (But this is because<br>
> I think implicit early flushes are a bug, not a feature, so I need to persuade<br>
> you of that first 😀 I wonder if the numbers are amenable to going through and<br>
> trying to work out which if any are genuinely helpful...)<br>
<br>
Sigh, we should probably make the helpful ones explicit and remove the rest.<br></blockquote><div><br></div><div>sgtm.</div><div><br></div><div>looking at all the callers, xputs isn't used much at all (21 calls in toys/) and most of them seem dubious. xprintf is a lot more popular (338), but -- though it's harder to tell because there are so many -- nothing particularly convincingly in need of a flush stood out.</div><div><br></div><div>i'm assuming this will be like FLAG, where we'll do it as we're touching things for other reasons?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
I'll add a cleaup pass to the todo heap...<br></blockquote><div><br></div><div>(you should probably keep that list checked in, or even stored as issues in the github bug tracker.)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Rob<br>
</blockquote></div></div>