Hello from Minneapolis: https://mstdn.jp/@landley/112078501045637288
Still _really_ fried from my move, but at least chipping away at the email backlog... On 3/11/24 12:27, enh wrote: > On Fri, Mar 8, 2024 at 5:43 PM Rob Landley <r...@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(). The commit comment on 3e0e8c687eee tried to explain: years ago xflush() got broken to have a "don't actually flush" argument, and thus didn't do what its name said anymore, so I renamed it to what it did (check errors and nothing more) and added a manual flush in the places that was passing in the "actual flush" argument. (So that change shouldn't have made it worse.) My original approach to FILE * was to just always flush (ala xprintf) but there were objections to that, which I said would open a can of micromanagement worms. Then I changed the default buffer type to be unbuffered instead, and there were objections to that, which I said would open a can of micromanagement worms... > but none of this > is relevant here --- the problem here is that we're not flushing _at > all_, so ToT watch is just broken. Micromanagement, yes. >> 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? Hmmm, yup looks like I was already fixing this command up to not use FILE * at all anymore (what is this, my fourth attempt to get away from FILE * buffering damage in a systematic way?) and got distracted partway through... > /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 > :-) The date on the file is February 10th, so I was working on this last month. Alas, my life's been a bit hectic recently. Trying to shovel out now... > 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. My brain is currently trying to filk the Steven Universe song "other friends" to be a demand about test cases and reproduction sequences, but reading the code I see the typo. > so the new patch (attached) is a better fix. but > that fixes the normal case and -b for me... Spraying down the code with manual fflush() calls strikes me as a code smell that means "this will break the next time anybody touches anything". But I mentioned still being fried from the move, which makes me extra irritable. (When did U-haul become a scam? That's new since the last time I used them...) Rob _______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net