[Toybox] Towards find cleanup

[Rob Landley]

On 04/10/2013 12:41:35 PM, Felix Janda wrote:
> Hello,
> 
> attached is some cleanup of the find toy inspired by Rob's (very cool)
> mails on how he proceeds when cleaning up toys. (and Isaac's recent
> partial cleanup of xzcat)
> 
> The patch contains a commit message.

Very nice. :)

Applied. Thank you.

> The cleanup was mainly mechanical and I still don't understand the toy
> well. (Before and after the cleanup) there seems to be a problem with
> the rule parsing/interpretation. For example
> 
> find . \( -type f -type f \) -o -type d
> 
> does not print directories.

Yup, it's an incomplete implementation. On the todo list. :)

> The toy also does not follow the whitespace conventions in toybox.  
> But I
> think that someone has scripts lying around to fix that.
> 
> I think that some function parameters should be made const and that
> the code could be made less repetitive.

I specify "static" because it allows the compiler to produce better  
code, but I've never found "const" to actually produce better code. The  
compiler's optimizer does liveness analysis, it can tell when you don't  
actually change a value after it's assigned to. In theory const lets  
the compiler do some of this across translation units, but that's what  
link time optimization is for these days.

In addition, const's syntax is unfortunately subtle when pointers are  
involved. Is the pointer what's const, is what it points to that's  
const (it binds left except at the left edge...), how about a pointer  
to a pointer, or pointer to an array...

I also haven't seen it interface well with the actual read-only  
sections ELF can produce. There's an -mebedded-data compiler flag that  
says to put data in the .rodata section. The main use of .rodata is to  
strings constants in there by default. Yet those strings aren't  
"const", you can assign a string constant to a regular char * without  
the compiler ever complaining. (And not being able to do so would  
pretty fundamentally break C.)

What const really seems like to me is a leakage from C++'s "public,  
private, protected" into C. It's a language facility that assumes  
programmers can't trust themselves or their successors to get it right.

So I tend to remove "const" from the code when I can. It doesn't  
justify its existence: if we're not supposed to modify the value from  
here, then don't do it. I trust the programmer to not suck, and I trust  
reviewers to catch it when they screw up.

Rob