On Mon, Sep 27, 2021 at 6:33 AM Isabella Basso isabellabdoamaral@usp.br wrote:
Split the test_int_hash function to keep its mainloop separate from arch-specific chunks, which are only compiled as needed. This aims at improving readability.
Tested-by: David Gow davidgow@google.com Signed-off-by: Isabella Basso isabellabdoamaral@usp.br
This looks good to me. It's possibly worth fixing up the changelog mixup between this and patch 3 if you send out a v3.
A minor suggestion re: commenting below, otherwise this is:
Reviewed-by: David Gow davidgow@google.com
-- David
lib/test_hash.c | 86 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 57 insertions(+), 29 deletions(-)
diff --git a/lib/test_hash.c b/lib/test_hash.c index d4b0cfdb0377..08fe63776c4f 100644 --- a/lib/test_hash.c +++ b/lib/test_hash.c @@ -56,6 +56,53 @@ fill_buf(char *buf, size_t len, u32 seed) } }
+/* Holds most testing variables for the int test */
It might be worth commenting what these variables actually are here, as it's pretty confusing on a quick read through.
Maybe something like:
+struct test_hash_params {
unsigned long long *h64;
/* Pointer to integer to be hashed. */
u32 h0;
/* Low 32-bits of integer to be hashed. */
u32 h1;
/* Arch-specific hash result. */
u32 h2;
/* Generic hash result. */
u32 (*hash_or)[33];
/* ORed hashes of given size (in bits) */
+};
+#ifdef HAVE_ARCH__HASH_32 +static bool __init +test_int__hash_32(struct test_hash_params *params) +{
params->hash_or[1][0] |= params->h2 = __hash_32_generic(params->h0);+#if HAVE_ARCH__HASH_32 == 1
if (params->h1 != params->h2) {pr_err("__hash_32(%#x) = %#x != __hash_32_generic() = %#x",params->h0, params->h1, params->h2);return false;}+#endif
return true;+} +#endif
+#ifdef HAVE_ARCH_HASH_64 +static bool __init +test_int_hash_64(struct test_hash_params *params, u32 const *m, int *k) +{
params->h2 = hash_64_generic(*params->h64, *k);+#if HAVE_ARCH_HASH_64 == 1
if (params->h1 != params->h2) {pr_err("hash_64(%#llx, %d) = %#x != hash_64_generic() = %#x",*params->h64, *k, params->h1, params->h2);return false;}+#else
if (params->h2 > *m) {pr_err("hash_64_generic(%#llx, %d) = %#x > %#x",*params->h64, *k, params->h1, *m);return false;}+#endif
return true;+} +#endif
/*
- 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,
@@ -69,19 +116,13 @@ static bool __init test_int_hash(unsigned long long h64, u32 hash_or[2][33]) { int k;
u32 h0 = (u32)h64, h1, h2;
struct test_hash_params params = { &h64, (u32)h64, 0, 0, hash_or }; /* Test __hash32 */
hash_or[0][0] |= h1 = __hash_32(h0);
hash_or[0][0] |= params.h1 = __hash_32(params.h0);#ifdef HAVE_ARCH__HASH_32
hash_or[1][0] |= h2 = __hash_32_generic(h0);-#if HAVE_ARCH__HASH_32 == 1
if (h1 != h2) {pr_err("__hash_32(%#x) = %#x != __hash_32_generic() = %#x",h0, h1, h2);
if (!test_int__hash_32(¶ms)) return false;
}-#endif #endif
/* Test k = 1..32 bits */@@ -89,37 +130,24 @@ test_int_hash(unsigned long long h64, u32 hash_or[2][33]) u32 const m = ((u32)2 << (k-1)) - 1; /* Low k bits set */
/* Test hash_32 */
hash_or[0][k] |= h1 = hash_32(h0, k);if (h1 > m) {pr_err("hash_32(%#x, %d) = %#x > %#x", h0, k, h1, m);
hash_or[0][k] |= params.h1 = hash_32(params.h0, k);if (params.h1 > m) {pr_err("hash_32(%#x, %d) = %#x > %#x", params.h0, k, params.h1, m); return false; } /* Test hash_64 */
hash_or[1][k] |= h1 = hash_64(h64, k);if (h1 > m) {pr_err("hash_64(%#llx, %d) = %#x > %#x", h64, k, h1, m);
hash_or[1][k] |= params.h1 = hash_64(h64, k);if (params.h1 > m) {pr_err("hash_64(%#llx, %d) = %#x > %#x", h64, k, params.h1, m); return false; }#ifdef HAVE_ARCH_HASH_64
h2 = hash_64_generic(h64, k);-#if HAVE_ARCH_HASH_64 == 1
if (h1 != h2) {pr_err("hash_64(%#llx, %d) = %#x != hash_64_generic() ""= %#x", h64, k, h1, h2);
if (!test_int_hash_64(¶ms, &m, &k)) return false;
}-#else
if (h2 > m) {pr_err("hash_64_generic(%#llx, %d) = %#x > %#x",h64, k, h1, m);return false;}-#endif #endif }
(void)h2; /* Suppress unused variable warning */ return true;}
-- 2.33.0