On 11/07/2012 01:37 AM, Michael W. Bombardieri wrote:
On Tue, Nov 06, 2012 at 08:47:27PM +0100, Edward Shishkin wrote:
Hello Michael,
Your changes look OK to me.
TBH, I don't like the fact that the pow is set to 1 by such tricky ways in
the mentioned nested loops: it might indicate mistakes in the original
code.
I agree. Something doesn't look right about the nested loops
not being entered. This could well point to the code being
incorrect. IMHO, the developer would not bother adding the
loops if he didn't want them to be used.
Generally I think that this hash is not heavily used: everyone uses either
r5, or tea hash, so...
I suppose this would raise the question, is it worth patching
yura_hash() or is it better to scrap the function altogether
if the two other hash functions are sufficient?
We can not scrap this, as it will break backward compatibility:
a number of people must be use it. We can only recommend,
or not recommend something..
I suggest to patch this, so it will be clear what is going on here.
You might want also to prepare similar patch for the kernel and
send it to Jeff.
Thanks,
Edward.
A??ked-by: Edward Shishkin <edward.shishkin@xxxxxxxxx>
Thanks,
Edward.
P.S. This is a git repository of Jeff Mahoney, the reiserfs maintainer, so
don't forget cc him ;)
- Michael
--- hashes.c Tue Nov 6 23:21:09 2012
+++ hashes_new.c Tue Nov 6 23:26:26 2012
@@ -175,37 +175,26 @@
u32 yura_hash (const signed char *msg, int len)
{
- int j, pow;
- u32 a, c;
- int i;
-
- for (pow=1,i=1; i < len; i++) pow = pow * 10;
-
- if (len == 1)
- a = msg[0]-48;
- else
- a = (msg[0] - 48) * pow;
-
- for (i=1; i < len; i++) {
- c = msg[i] - 48;
- for (pow=1,j=i; j < len-1; j++) pow = pow * 10;
- a = a + c * pow;
- }
-
- for (; i < 40; i++) {
- c = '0' - 48;
- for (pow=1,j=i; j < len-1; j++) pow = pow * 10;
- a = a + c * pow;
- }
-
- for (; i < 256; i++) {
- c = i;
- for (pow=1,j=i; j < len-1; j++) pow = pow * 10;
- a = a + c * pow;
- }
-
- a = a << 7;
- return a;
+ int i, j, pow;
+ u32 a;
+
+ a = msg[0] - 48;
+ if (len != 1) {
+ for (i = 1, pow = 1; i < len; i++)
+ pow *= 10;
+ a *= pow;
+ }
+ for (i = 1; i < len; i++) {
+ for (j = i, pow = 1; j < len - 1; j++)
+ pow *= 10;
+ a += pow * (msg[i] - 48);
+ }
+ for (; i < 40; i++)
+ a += '0' - 48;
+ for (; i < 256; i++)
+ a += i;
+ a <<= 7;
+ return a;
}
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
