[Toybox] Towards find cleanup

Felix Janda felix.janda at posteo.de
Wed Apr 10 12:29:01 PDT 2013


Tim, thanks for your thourough review.

Some (rather selective) comments:

Regarding the "== 0" vs "!" thing: Maybe I was too eager here. I think
that it does not make a great difference in readability but should be
uniform along in all the toys. Grepping in toys/posix "!" is used
much more often but there are some occurences of "== 0".  Let's hear
Rob...

On 04/10/13 at 11:55am, Tim Bird wrote:
> snip
> >  	if (filter->op==OP_OR) {
> > -		result = evaluate(filter->next, node, fnext);
> > -		result2 = evaluate(*fnext, node, fnext);
> > -		if (result) {
> > -			return result;
> > -		} else {
> > -			if (result2) {
> > -				return result2;
> > -			} else {
> > -				return 0;
> > -			}
> > -		}
> > +		return evaluate(filter->next, node, fnext) || evaluate(*fnext, node, fnext);
> This will have different side effects than the original code.
> The original code evaluates both sides, and then returns the 'or' operation
> on the results.  The new one will (of course) do short-circuit evaluation of
> the '||',  meaning that sometimes the second node won't be evaluated.
> I believe that at one time, I actually had it coded that way, but for some
> reason or other I went back to forcing the evaluation of both nodes.
> Unfortunately, I can't remember the reason why I went back.  I would have expected
> regular find to do short-circuit evaluation of expressions with 'or' operations,
> so I'd be happy if this were correct.  I just have a nagging feeling that
> there was some reason I couldn't do it this way.  I could be completely wrong,
> though.  So unless testing reveals the need for evaluating both sides of the
> 'or', this should be fine.
> 
> Do you know if regular find does short-circuit evaluation?
> What would regular find do with
>   'find . -exec test1_with_side_effects {} \; -o -exec test2_with_side_effects {} \;'
> ?

POSIX says that these short-circuit evaluations shall be applied. (The
same applies for -a.)

Felix



More information about the Toybox mailing list