[Toybox] [PATCH] chvt: remove old workarounds.

[Rob Landley]

On 9/25/21 3:21 PM, enh wrote:
> 
> 
> On Sat, Sep 25, 2021 at 9:30 AM Rob Landley <rob at landley.net
> <mailto:rob at landley.net>> wrote:
> 
>     On 9/24/21 7:10 PM, enh via Toybox wrote:
>     > We don't need the fd=fd hack for GCC 3.4 in 2021,
> 
>     Yeah, but we still needed it in 2018.
> 
> 
> do you have a similar 7 year rule for the toolchain used to build toybox, on the
> assumption that embedded folks especially are likely to be on gcc 2.95^W^Wold
> compilers?

Not for warnings, no. It has to work, it doesn't have to be pretty. :)

>     *cracks knuckles* The power of grep '[ ,]\([a-zA-Z0-9_][a-zA-Z0-9_]*\) *= *\1'
>     *.c lib/*.c toys/*/*.c compels you!
> 
>     Noise, noise, noise...
> 
>     toys/net/sntp.c:  unsigned long long *pktime = (void *)toybuf, now, then, before
>     = before;
>     toys/other/chvt.c:  int vtnum, fd = fd;
>     toys/other/switch_root.c:  int console = console; // gcc's "may be used"
>     warnings are broken.
>     toys/other/watch.c:  unsigned width, height, i, cmdlen, len, xx = xx, yy = yy,
>     active = active;
>     toys/posix/uudecode.c:  int ofd, idx = 0, m = m, n;
> 
>     I could definitely clean that regex up more...
> 
> 
> yeah, the trouble with this self-assignment pattern is that although it shuts up
> that compiler, it annoys static analysis tools and makes it harder to find the
> interesting stuff (like the sscanf off-by-one) amongst the noise.

I'm up for removing 'em. GCC was buggy and annoying for MANY YEARS. It is now
(apparently) less buggy and annoying. If that's recent enough that
musl-cross-make is churning out a usable toolchain (gcc 9.2.0) then I can
regression test the various architecture targets with that.

> i know your argument about not wanting the compiler to emit "provably useless"
> assignments, but since Android builds everything with stack variables allocated
> to 0 anyway [and lets the backend sort out which ones _it_ can prove aren't
> needed], this pattern isn't helpful to us. and it confuses _human_ readers of
> the code. i get quite a lot of "i found a bug in toybox" feedback from
> self-assignment.

The first few I commented, but alas it became too common a pattern to keep that up.

> 
>     > and <linux/vt.h> has
>     > given names to the two magic numbers since at least Linux 2.4.0...
> 
>     Heh, back when I was trying not to include linux/*.h at all, before I can up
>     with the "command can include it, headers shouldn't" policy.
> 
> yeah, speaking of style, i felt like 2021 rob would have `int
> vt=atoi(*toys.optargs), fd;` rather than the separate line for the atoi(), but
> wasn't sure so i left that even though i touched both lines.

Eh, it's not something I feel strongly about. (It's a bikeshedding level of
aesthetics.)

> perhaps more
> interestingly, i wondered whether atoi() is quite right there: should `chvt foo`
> change to vt 0?

That's what the host one does. (Just checked under strace to confirm.)

> (but since Android didn't use this, i had no opinion and ignored
> it. i'm literally only here because it was nearby the static analyser's
> complaint about scanf, and i thought "ideally i'd never have to see this again"
> :-) )

Indeed. I don't really use it myself either. (It was a contribution. I haven't
removed it because it's not _wrong_, but...)

I had to deal with a server recently that was panicing (because binary only
nvidia driver, of course, but the interesting stuff was scrolled off by
collateral ext4 panics), and shift-pgup wasn't working to see the interesting
info, and I googled to see if I was using the right key combo and found:

https://www.phoronix.com/scan.php?page=news_item&px=Linux-5.9-Drops-Soft-Scrollback

Sigh. (And of COURSE it doesn't have a real serial port on there, no USB doesn't
count. So I taught the admin to install netconsole and they bought a raspberry
pi to put in the server room to receive it, and THEN we could debug it...)

>     (Back in 2008 Steve Jobs wasn't dead yet and smartphones were brand new, so
>     "mac/darwin might take over the world or at least meat shield us from the
>     windopoly" was still a viable concept. lib/portability.h showed up in jan 2007
>     but sticking linux/ headers in there didn't really help, and portability.c
>     didn't show up until 2012...)
> 
> for all i complain about the pain of building for macOS (and especially the lack
> of convenient cross-compilation), i was genuinely surprised they didn't have
> POSIX timers. especially since they did finally add CLOCK_MONOTONIC for
> clock_gettime(). maybe in 2031 we can delete this...

Eh, I'm not impressed by Apple's trajectory. Ever since Jobs got sick they've
been recycling the same designs/circuits, and don't even fix the obvious flaws.
Louis Rossman (who repairs macbooks for a living) did a roundup of that in 2018:

  https://www.youtube.com/watch?v=AUaJ8pDlxi8

Then in 2019 Johnny Ive malfunctioned due to a lack of input:

  https://www.theverge.com/2019/11/28/20986838/jony-ive-last-day-apple and

Which the WSJ blamed on Tim Cook, who strikes me as basically John Sculley 2.0,
a standard issue "spend less and charge more" CEO who sees engineering as just
one more expense to cut. Admittedly defending your turf and squeezing blood out
of stones works a lot better when you start as king of the hill in a mature
technology market rather than an underdog in a rapidly developing area, but it's
still fundamentally riding down an installed base, not building anything.

What new concepts has Apple introduced in the past 5 years? They bought "Beats
by Dre" and sold wifi earbuds for $200. They took the GPS tracker out of the
phone and made it standalone. TSMC and Samsung got 5 nanometer fabs working and
Apple fed an arm design into them (moving off x86 to arm for the same reason
chromebooks did years ago, taking advantage of a die size shrink to make their
contribution seem clever).

Haven't heard anything better about them on the software side, but mostly I just
hear complaints every time a new OS version comes out. Dunno how much that
really means...

>     I do note that:
> 
>     - if (-1 != (fd = open(*cc, O_RDWR))) break;
>     + if ((fd = open(*cc, O_RDWR)) != -1) break;
> 
>     Was alas good advice that never quite soaked in. (If you typo x == 1 as x = 1 it
>     compiles. If you typo 1 = x it does not. But for constructs like this, "do the
>     thing, then error check the thing" makes more sense to human programmers...)
> 
> this case is fine even in non-yoda style :-)

It's always fine if you don't screw up. :)

> cc     x.c   -o x
> x.c: In function ‘main’:
> x.c:7:34: error: lvalue required as left operand of assignment
>     7 |   if ((fd = open("foo", O_RDWR)) = -1) return 1;
>       |                                  ^

Sure, but constant on the left sometimes and constant on the right other times
is not easier to read than being consistent.

*shrug* As I said, it didn't take...

Rob