On Wed, Apr 19, 2023 at 8:44 PM Rob Landley <r...@landley.net> wrote:
> On 4/19/23 16:02, enh wrote: > > On Fri, Apr 14, 2023 at 8:49 PM Rob Landley <r...@landley.net > > <mailto:r...@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.) > you're the one who forked this from the original discussion on the specific line of code in question :-P https://github.com/landley/toybox/commit/aa88571a6b847a96bb8ee998a9868c5a1bdb3a6e#r108474092 the whole reason i suggested static_assert is because at the moment all the "protection" we have is a comment. > > 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. correct. > But I'm unaware of such a check...? > exactly :-P 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". > 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 >
_______________________________________________ Toybox mailing list Toybox@lists.landley.net http://lists.landley.net/listinfo.cgi/toybox-landley.net
