Re: [PATCH v3 07/10] : Add support for architecture-specific functions

Geert Uytterhoeven
2016-05-29T07:57:15Z

Hi George,

I see this has been applied in the mean time, but improvements
never hurt...

On Sat, May 28, 2016 at 9:57 PM, George Spelvin
<linux@sciencehorizons.net> wrote:
> --- /dev/null
> +++ b/lib/test_hash.c
> @@ -0,0 +1,250 @@

> +/*
> + * Test the various integer hash functions. h64 (or its low-order bits)
> + * is the integer to hash. hash_or accumulates the OR of the hash values,
> + * which are later checked to see that they cover all the requested bits.
> + *
> + * Because these functions (as opposed to the string hashes) are all
> + * inline, the code being tested is actually in the module, and you can
> + * recompile and re-test the module without rebooting.
> + */
> +static bool __init
> +test_int_hash(unsigned long long h64, u32 hash_or[2][33])
> +{
> + int k;
> + u32 h0 = (u32)h64, h1, h2;
> +
> + /* Test __hash32 */
> + hash_or[0][0] |= h1 = __hash_32(h0);
> +#ifdef HAVE_ARCH__HASH_32
> + hash_or[1][0] |= h2 = __hash_32_generic(h0);

__hash_32_generic() always exist, right?
So you can reduce #ifdefery and increase compile coverage by dropping the
#ifdef ...

> +#if HAVE_ARCH__HASH_32 == 1
> + if (h1 != h2) {

... and replacing this test by

if (identical_hashes && (h1 != h2))

with identical_hashes a static const bool initialized depending on
HAVE_ARCH__HASH_32.

> + pr_err("__hash_32(%#x) = %#x != __hash_32_generic() = %#x",
> + h0, h1, h2);
> + return false;
> + }
> +#endif
> +#endif

The same comment applies to the other hunks.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds