RE: Performance improvements to key comparison functions

["David Dabbs" <david@dabbs.net>]

Thanks for looking over the code and for your responses.

> Nikita wrote
> Note that comparisons in C (like (v1 > v2)) are not guaranteed to return 0
> or _1_ as a result, but simply zero, or _non-zero_.

Is the information here (http://www.lysator.liu.se/c/c-faq/c-8.html)
incorrect then? Specifically,

---8<  snip

It is true (sic) that any nonzero value is considered true in C, but this
applies only "on input", i.e. where a boolean value is expected.  When a
boolean value is generated by a built-in operator, it is guaranteed to be 1
or 0.  Therefore, the test 


	if((a == b) == TRUE)

will work as expected (as long as TRUE is 1), but it is obviously silly.  In
general, explicit tests against TRUE and FALSE are undesirable, because some
library functions (notably isupper, isalpha, etc.) return, on success, a
nonzero value which is not necessarily 1.  (Besides, if you believe that
"if((a == b) == TRUE)" is an improvement over "if(a == b)", why stop there?
Why not use "if(((a == b) == TRUE) == TRUE)"?)  A good rule of thumb is to
use TRUE and FALSE (or the like) only for assignment to a Boolean variable
or function parameter, or as the return value from a Boolean function, but
never in a comparison. 

The preprocessor macros TRUE and FALSE are used for code readability, not
because the underlying values might ever change.  (See also questions 1.7
and 1.9.) 

References: K&R I Sec. 2.7 p. 41; K&R II Sec. 2.6 p. 42, Sec. A7.4.7 p. 204,
Sec. A7.9 p. 206; ANSI Secs. 3.3.3.3, 3.3.8, 3.3.9, 3.3.13, 3.3.14, 3.3.15,
3.6.4.1, 3.6.5; Achilles and the Tortoise. 

---8<  snip


> In the same vein, "isunique" argument of
> znode_contains_key_strict_new() is not guaranteed to be 0 or 1
> which renders this function wrong.
> That said, I think that proper way to test your functions is to plug
> them into kernel reiser4 version and run some CPU intensive tests.

I thought I noted in "System Impacts" that when isunique is initialized by
ORing the flag that the initialization expression must change to be a
Boolean result of the OR test. Something like 

	isunique = ((h->flags & CBK_UNIQUE) == CBK_UNIQUE);

That may have not made it in.
 
As to testing in the kernel, I completely agree. Before going there I
thought I'd get community feedback to see whether there was something subtle
I had missed.

Thanks again,

David