<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Sep 25, 2021 at 9:30 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 9/24/21 7:10 PM, enh via Toybox wrote:<br>
> We don't need the fd=fd hack for GCC 3.4 in 2021,<br>
<br>
Yeah, but we still needed it in 2018.<br></blockquote><div><br></div><div>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?</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">
*cracks knuckles* The power of grep '[ ,]\([a-zA-Z0-9_][a-zA-Z0-9_]*\) *= *\1'<br>
*.c lib/*.c toys/*/*.c compels you!<br>
<br>
Noise, noise, noise...<br>
<br>
toys/net/sntp.c: unsigned long long *pktime = (void *)toybuf, now, then, before<br>
= before;<br>
toys/other/chvt.c: int vtnum, fd = fd;<br>
toys/other/switch_root.c: int console = console; // gcc's "may be used"<br>
warnings are broken.<br>
toys/other/watch.c: unsigned width, height, i, cmdlen, len, xx = xx, yy = yy,<br>
active = active;<br>
toys/posix/uudecode.c: int ofd, idx = 0, m = m, n;<br>
<br>
I could definitely clean that regex up more...<br></blockquote><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>(not right now, but at some point i'd like to turn on the self-assignment warning for Android. it's more usually a mistake than rob landley arguing that the code that follows is correct, just not obviously so :-) [i forgot to include in my commit message that i also removed the redundant !*cc check, which seems like the person who originally wrote the code didn't quite trust that fd would definitely be -1 by this point either! i've certainly been accused of writing overly "clever" code before, but i do at least try not to check in code that i don't at least understand myself :-) as someone i used to work with used to say: "i don't know much about future me, but it's a safe bet i won't be getting any cleverer with time".])</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 <linux/vt.h> has<br>
> given names to the two magic numbers since at least Linux 2.4.0...<br>
<br>
Heh, back when I was trying not to include linux/*.h at all, before I can up<br>
with the "command can include it, headers shouldn't" policy.<br></blockquote><div><br></div><div>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. perhaps more interestingly, i wondered whether atoi() is quite right there: should `chvt foo` change to vt 0? (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" :-) )</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">
(Back in 2008 Steve Jobs wasn't dead yet and smartphones were brand new, so<br>
"mac/darwin might take over the world or at least meat shield us from the<br>
windopoly" was still a viable concept. lib/portability.h showed up in jan 2007<br>
but sticking linux/ headers in there didn't really help, and portability.c<br>
didn't show up until 2012...)<br></blockquote><div><br></div><div>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...</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 do note that:<br>
<br>
- if (-1 != (fd = open(*cc, O_RDWR))) break;<br>
+ if ((fd = open(*cc, O_RDWR)) != -1) break;<br>
<br>
Was alas good advice that never quite soaked in. (If you typo x == 1 as x = 1 it<br>
compiles. If you typo 1 = x it does not. But for constructs like this, "do the<br>
thing, then error check the thing" makes more sense to human programmers...)<br></blockquote><div><br></div><div>this case is fine even in non-yoda style :-)</div><div><br></div><div>cc x.c -o x<br>x.c: In function ‘main’:<br>x.c:7:34: error: lvalue required as left operand of assignment<br> 7 | if ((fd = open("foo", O_RDWR)) = -1) return 1;<br> | ^<br></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>
_______________________________________________<br>
Toybox mailing list<br>
<a href="mailto:Toybox@lists.landley.net" target="_blank">Toybox@lists.landley.net</a><br>
<a href="http://lists.landley.net/listinfo.cgi/toybox-landley.net" rel="noreferrer" target="_blank">http://lists.landley.net/listinfo.cgi/toybox-landley.net</a><br>
</blockquote></div></div>