<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Apr 19, 2023 at 8:44 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 4/19/23 16:02, enh wrote:<br>
> On Fri, Apr 14, 2023 at 8:49 PM 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 4/14/23 18:49, enh wrote:<br>
> > > i don't like assert(), but **static_assert** is really useful for<br>
> things like<br>
> > > this where you want to say "this code makes an assumption that you<br>
> can test at<br>
> > > compile time".<br>
> ><br>
> > Compile time is the time to care about that sort of thing, yes.<br>
> ><br>
> > Kinda wonder about portability on weirdness like qnx (or the guy in<br>
> email who's<br>
> > asking about uclibc-ng). I suspect any such asserts would be a<br>
> CFG_DEBUG option<br>
> > maybe in portability.c? Hmmm...<br>
> ><br>
> > they haven't made other complaints about C11, so presumably static_assert is<br>
> > fine too?<br>
> <br>
> I'm not worried about it erroring, I'm worried about it potentially triggering<br>
> in who knows what linker environment possibly producing mach-o binaries.<br>
> <br>
> i think you've mixed up two different threads here...<br>
> <br>
> static_assert is a compile-time check. there's currently a dependency in the<br>
> checksum code on sizeof(toybox) being < 5k-ish.<br>
<br>
Really? Checking a compile time constant at runtime doesn't sound like my style.<br>
(As in I'd have removed it during cleanup if I'd noticed, unless I _expect_ the<br>
test to optimize out and be a de-facto assert.)<br></blockquote><div><br></div><div>you're the one who forked this from the original discussion on the specific line of code in question :-P</div><div><br></div><div><a href="https://github.com/landley/toybox/commit/aa88571a6b847a96bb8ee998a9868c5a1bdb3a6e#r108474092" rel="noreferrer" target="_blank">https://github.com/landley/toybox/commit/aa88571a6b847a96bb8ee998a9868c5a1bdb3a6e#r108474092</a><br></div><div><br></div><div>the whole reason i suggested static_assert is because at the moment all the "protection" we have is a comment.</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">
> static_assert can help you tell<br>
> the compiler to fail at compile time if that's not true. (and nothing ends up in<br>
> the binary.)<br>
<br>
I could have done #if sizeof(walrus) > 1234 around #error in portability.[ch]<br>
years ago if I'd needed to, the assert presumably is just syntactic sugar for<br>
that. </blockquote><div><br></div><div>correct.</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">But I'm unaware of such a check...?<br></blockquote><div><br></div><div>exactly :-P</div><div><br></div><div>to go back to the beginning since this thread got far too confused: "if you don't do this because the #if .. #error dance is too annoying/verbose, static_assert lets you say this very concisely, and you're already requiring C11 anyway".</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">
Checksum code... Do you mean libbuf? Not seeing such a check...<br>
<br>
There's a check that ping -s is small enough to fit in toybuf but that's the<br>
optstr providing a boundary for a command line argument... Lots of little<br>
implicit limitations due to using toybuf, such as ls -C maxing out at 1024<br>
columns...<br>
<br>
Not spotting it with a grep that includes sizeof. It could be a variable<br>
assignment that then gets tested later, but I'm not finding it at the moment.<br>
<br>
> alignas, on the other hand, _does_ cause changes to the elf/mach-o files. but<br>
> apple switched to clang before we did :-) (i also tested just now --- alignas<br>
> works as expected on macOS.)<br>
<br>
Ah, you're proposing the assert without the alignas. Presumably to (somehow?)<br>
check that none of the libc structs I'm throwing in toybuf gets bigger than<br>
toybuf because of library version skew. (Is there an easier way than putting an<br>
assert on every single such assignment? I generally check them and don't use<br>
toybox if it would take up half of it, and I also tend to check if the size of<br>
the glibc one, which is GOING to be the pig, has changed in the past 15 years...)<br>
<br>
Let's see, demo_editline throws struct termios in there, httpd and netcat are<br>
using it as struct sockaddr, ping is using it as struct icmphdr... Ah, there's a<br>
pingchksum() using toybuf, is that what you meant? Again, that size check is on<br>
the size of the command line argument supplied to this run...<br>
<br>
All the sizeof(toybuf) tests SHOULD be comparing against something that varies<br>
at runtime...<br>
<br>
> Sigh. I tried to make it gracefully fall back, and you guys are sending kill<br>
> signals to anything that uses a system call you don't recognize.<br>
> <br>
> like i say every time you complain: because the alternative was worse. we<br>
> started off just failing the syscalls, but *way* too much code doesn't check, or<br>
> has untested error paths that don't actually work, or has error paths that do<br>
> "work" but just hide the failure and make it impossible to debug. if you're<br>
> trying to do this for a single project, that would be an option. for an entire<br>
> operating system, not so much.<br>
<br>
Indeed. You're shipping a billion unadministered unix systems per year with<br>
full-room audio pickup, GPS tracking, and a camera pointed at the user's face in<br>
normal operation, through which their users regularly do financial transactions.<br>
<br>
Your paranoia level is quite understandably a bit high.<br>
<br>
> > if we want to go this route, we'll probably have to go with something like a<br>
> > signal handler for SIGSYS and a boolean. or a weak declaration of the<br>
> > copy_file_range() *function*? no, because that's useless for the static<br>
> linking<br>
> > we're concerned about.<br>
> ><br>
> > oh, wait... we should just be able to use android_get_device_api_level()...<br>
> ><br>
> > i think something like this should work?<br>
> <br>
> Sigh, lemme finish and check in the fs_type_name() redo. (So many dirty files in<br>
> my tree. Closing tabs...)<br>
> <br>
> well, i should try to test this first... :-) if nothing else, it occurred to me<br>
> since sending it that we shouldn't check the system property on _every_ call,<br>
> and should cache that.<br>
<br>
Ready to test a patch when you've got an update...<br>
<br>
Rob<br>
</blockquote></div></div>