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
_______________________________________________
Toybox mailing list
[email protected]
http://lists.landley.net/listinfo.cgi/toybox-landley.net
