On 17/02/2021 17:49, Mark Wielaard wrote:
I think both the comment and the warning message are a little
misleading.
The comment should really read "Change state from CLEANING to
NO_RESIZING". You are right that NO_RESIZING is just zero, but still
removing it seems to make the code less clear.
All this is slightly confusing since it is doing "double" xor.
First it does an OLD_STATE_BITS ^ NEW_STATE_BITS and then a
atomic_fetch_xor of the result of that xor with the resizing_state
variable, Which is expected to be OLD_STATE, so the bits that are left
should be the NEW_STATE bits after the double xor.
This is how I understood the code. Given that all of the
atomic_fetch_xor_explicit() calls in that file preceded by a comment
explaining what they do, it might be useful to have a function
change_resizing_state(htab, from, to, memory_order) instead and remove
the comments. That also works around the clang warning of course, but
those are not the only places where resizing_state is modified, so it's
weird in a different way.
Could you see if something like this works around the warning?
diff --git a/lib/dynamicsizehash_concurrent.c b/lib/dynamicsizehash_concurrent.c
index 2d53bec6..0283eea1 100644
--- a/lib/dynamicsizehash_concurrent.c
+++ b/lib/dynamicsizehash_concurrent.c
@@ -160,10 +160,10 @@ insert_helper (NAME *htab, HASHTYPE hval, TYPE val)
}
}
-#define NO_RESIZING 0u
-#define ALLOCATING_MEMORY 1u
-#define MOVING_DATA 3u
-#define CLEANING 2u
+#define NO_RESIZING 0x0
+#define ALLOCATING_MEMORY 0x1
+#define MOVING_DATA 0x3
+#define CLEANING 0x2
Nope, same result. :(
--
Red Hat GmbH, http://www.de.redhat.com/, Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Michael O'Neill, Tom Savage, Eric
Shander
