<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Nov 8, 2024 at 5:56 PM 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 11/8/24 08:10, enh wrote:<br>
> hmm... thinking about this more we don't actually need this condition<br>
> <br>
>        else if (sizeof(long)==8 && bytes==8) data = *(unsigned long *)p;<br>
> <br>
> because we already check that we don't let you set bytes to 8 if you're on<br>
> ILP32 (which is good, because _that_ would be a bug).<br>
> <br>
> so we can just write<br>
> <br>
>        else data = *(unsigned long *)p;<br>
<br>
It was a compile time optimization to let dead code elimination yank a <br>
tiny amount of code because sizeof(long) is a compile type constant. I <br>
mostly don't do that level of micromanagement anymore because busybox <br>
exists and compilers sometimes get smarter. Well, they change their <br>
optimizations around a lot. Somewhat randomly. And get lucky sometimes.<br>
<br>
> here (and in the other "copy" of this if above) and make this go away with<br>
> less code? if we do that, we can remove the QUIET from data completely<br>
> (because at least with `make defconfig` gcc 13 doesn't complain about this<br>
> code at all).<br>
<br>
I already applied the patch.<br>
<br>
And I should try the "commit to a branch then delete the branch" trick <br>
to close <a href="https://github.com/landley/toybox/issues/520" rel="noreferrer" target="_blank">https://github.com/landley/toybox/issues/520</a> now I've lost web <br>
login access.<br>
<br>
I hadn't gotten around to that until you poked me because last I checked <br>
LLVM was setting __gnuc__ so this didn't bite in NDK testing, and I <br>
thought they were talking about qnx or something. Which I should deal <br>
with before cutting a release but isn't otherwise time critical because <br>
I don't have a test environment for it so never know when I break it (or <br>
fix it)...<br>
<br>
I'm guessing one of the updates finally yanked that define?<br></blockquote><div><br></div><div>no, clang has defined __GNUC__ since before Android switched from gcc. i think this is just the first time that clang has agreed with gcc about the questionability of a piece of code. (there are very few QUIETs in the tree still.)</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 if we do _that_, then i wonder whether we should keep clang on the<br>
> "no-op QUIET" path until/unless we really do need it, to force us to<br>
> continue to look at and think about these things? there are very few uses<br>
> of QUIET, and it does seem like something we should avoid...<br>
<br>
It would be nice if gcc would just stop producing unsilenceable false <br>
positives because it can't understand the relationship between two <br>
variables. Alas, 7 year time horizon also means building with older gcc, <br>
and a LOT of cross compilers aren't the latest and greatest because <br>
updating just causes pain when upstream doesn't know you exist and isn't <br>
happy to hear from you...<br></blockquote><div><br></div><div>to be fair, this one is pretty complicated --- you have to understand a string literal amongst other things. (which is probably why this is the first one where even clang says "i dunno about 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">
Anyway, commit 542f62759a779<br></blockquote><div><br></div><div>thanks! i've updated AOSP and the tests are passing.</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>