[Toybox] sizeof(toybuf)

[Rob Landley]

On 4/19/23 16:02, enh wrote:
> On Fri, Apr 14, 2023 at 8:49 PM Rob Landley <rob at landley.net
> <mailto:rob at landley.net>> wrote:
> 
>     On 4/14/23 18:49, enh wrote:
>     >     > i don't like assert(), but **static_assert** is really useful for
>     things like
>     >     > this where you want to say "this code makes an assumption that you
>     can test at
>     >     > compile time".
>     >
>     >     Compile time is the time to care about that sort of thing, yes.
>     >
>     >     Kinda wonder about portability on weirdness like qnx (or the guy in
>     email who's
>     >     asking about uclibc-ng). I suspect any such asserts would be a
>     CFG_DEBUG option
>     >     maybe in portability.c? Hmmm...
>     >
>     > they haven't made other complaints about C11, so presumably static_assert is
>     > fine too?
> 
>     I'm not worried about it erroring, I'm worried about it potentially triggering
>     in who knows what linker environment possibly producing mach-o binaries.
> 
> i think you've mixed up two different threads here...
> 
> static_assert is a compile-time check. there's currently a dependency in the
> checksum code on sizeof(toybox) being < 5k-ish.

Really? Checking a compile time constant at runtime doesn't sound like my style.
(As in I'd have removed it during cleanup if I'd noticed, unless I _expect_ the
test to optimize out and be a de-facto assert.)

> static_assert can help you tell
> the compiler to fail at compile time if that's not true. (and nothing ends up in
> the binary.)

I could have done #if sizeof(walrus) > 1234 around #error in portability.[ch]
years ago if I'd needed to, the assert presumably is just syntactic sugar for
that. But I'm unaware of such a check...?

Checksum code... Do you mean libbuf? Not seeing such a check...

There's a check that ping -s is small enough to fit in toybuf but that's the
optstr providing a boundary for a command line argument... Lots of little
implicit limitations due to using toybuf, such as ls -C maxing out at 1024
columns...

Not spotting it with a grep that includes sizeof. It could be a variable
assignment that then gets tested later, but I'm not finding it at the moment.

> alignas, on the other hand, _does_ cause changes to the elf/mach-o files. but
> apple switched to clang before we did :-) (i also tested just now --- alignas
> works as expected on macOS.)

Ah, you're proposing the assert without the alignas. Presumably to (somehow?)
check that none of the libc structs I'm throwing in toybuf gets bigger than
toybuf because of library version skew. (Is there an easier way than putting an
assert on every single such assignment? I generally check them and don't use
toybox if it would take up half of it, and I also tend to check if the size of
the glibc one, which is GOING to be the pig, has changed in the past 15 years...)

Let's see, demo_editline throws struct termios in there, httpd and netcat are
using it as struct sockaddr, ping is using it as struct icmphdr... Ah, there's a
pingchksum() using toybuf, is that what you meant? Again, that size check is on
the size of the command line argument supplied to this run...

All the sizeof(toybuf) tests SHOULD be comparing against something that varies
at runtime...

>     Sigh. I tried to make it gracefully fall back, and you guys are sending kill
>     signals to anything that uses a system call you don't recognize.
> 
> like i say every time you complain: because the alternative was worse. we
> started off just failing the syscalls, but *way* too much code doesn't check, or
> has untested error paths that don't actually work, or has error paths that do
> "work" but just hide the failure and make it impossible to debug. if you're
> trying to do this for a single project, that would be an option. for an entire
> operating system, not so much.

Indeed. You're shipping a billion unadministered unix systems per year with
full-room audio pickup, GPS tracking, and a camera pointed at the user's face in
normal operation, through which their users regularly do financial transactions.

Your paranoia level is quite understandably a bit high.

>     > if we want to go this route, we'll probably have to go with something like a
>     > signal handler for SIGSYS and a boolean. or a weak declaration of the
>     > copy_file_range() *function*? no, because that's useless for the static
>     linking
>     > we're concerned about.
>     >
>     > oh, wait... we should just be able to use android_get_device_api_level()...
>     >
>     > i think something like this should work?
> 
>     Sigh, lemme finish and check in the fs_type_name() redo. (So many dirty files in
>     my tree. Closing tabs...)
> 
> well, i should try to test this first... :-) if nothing else, it occurred to me
> since sending it that we shouldn't check the system property on _every_ call,
> and should cache that.

Ready to test a patch when you've got an update...

Rob