[Toybox] More find cleanup

[Rob Landley]

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