On 04/20/2013 01:39:10 PM, Felix Janda wrote:
Hello,
some more find cleanup in an attached patch.
The main change was to make some code in build_filter_list() less
repetitive using a suitable struct and a loop.
Problem: you defined a struct locally, then used that structure
definition in GLOBALS. This works for buliding find itself, but breaks
for building every other command, because they haven't got this
structure definition.
The reason is generated/globals.h defines "struct mycommand_struct" for
each command, containing the body of your GLOBALS() block. This file
ends with union global_union containing all those structs. If there's
an unknown structure type in one of them that's not defined in headers
#included before globals.h, the compiler can't determine the size of
the union, and barfs when anything tries to actually use it.
In toys.h we include generated/globals.h near the end, after lib/lib.h.
So we can use any of the lib.h types in GLOBALS(), but can't use a
locally defined type. In main.c it declares the actual instances of
these variables, and then the TT macros (defined in globals.h based on
the FOR_command defines) expand to this.command thus referencing the
right structure type out of the global union.
The workaround is to use a void * in globals and typecast it when you
need to. (In C that's almost never, it auto-typecasts void pointer
assignments in both directions. I keep thinking it's only one
direction, but that's C++'s static typechecking paranoia.) Or since
it's a list use one of the list types, and again typecast it when you
need it. If all that's actually stored is a pointer, the pointer type
doesn't hugely matter, it's the same 4/8 bytes.
If you need one instance of a struct, and for some reason you can't
expand those contents into TT, you can use a chunk of toybuf to hold
it, or declare an instance on the stack at the start of command_main()
and store a void * to it in GLOBALS. (The lifetime of main's locals are
the lifetime of your command. I'd have just had TT be a pointer to a
struct on the stack in main() but some build environments have a small
stack. Yes, I used to care more about nommu than I do now. :)
(If there's a good enough reason you can throw the structure definition
into lib/lib.h but the rule of thumb there is "used by more than one
command".)
The command line parsing
is however now less efficient since after an argument has been tested
for equality with different strings like "-mtime" and the
corresponding
type (CHECK_MTIME) has been saved, the saved integer has again to be
checked for equality with stuff to find out what further command line
parsing is necessary.
An integer check is trivial. String comparisons do an order of
magnitude more work and evict cache lines in the process. On processors
with multiple execution units (on x86 that's everything since the
original pentium) an extra integer operation may even use an otherwise
idle execution unit so literally take no time at all. (Branches can be
expensive, it depends.)
I'm sure I've written up the whole spiel about "branch prediction and
register renaming, and why the embedded world chooses not to" at some
point. If I was more awake I could probably not only find it but work
in a joke about at quoted block's structural parallels to the full
title of The Hobbit. Lemme know if you're curious enough for me to redo
it here...
Apart from this I moved around some variables and got rid of the
unnecessary index "i" in build_filter_list().
Woot.
Felix
BTW: POSIX-1.2008 TC1 is out
I might be happy about this if they hadn't deleted the original
Posix-2008 to post 5 years of updates all at once (Posix-2001 to
Posix-2008 was only 7 years) without an obvious changelog. (It says the
rationale has one, but that's just "why we removed tar and cpio in
favor of a pax command nobody will ever use, why we decided not to
include chroot, su, mknod, login, dc because who uses _that_..." All
2008 decisions, no 2013 diffs.)
Instead, I'm kind of annoyed about it. Luckily, I downloaded the old
one.
(Version control, guys. It's a thing.)
Rob
_______________________________________________
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net
