Remove misleading XOR

[Mark Wielaard]

On Wed, 2021-02-17 at 11:12 +0100, Timm Bäder via Elfutils-devel wrote:
> Hi,
> 
> I'm mainly leaving this patch here to discuss what to do about it.
> Clang complains because of -Wxor-used-as-pow. So of course one
> possibility is to use -Wno-xor-used-as-pow. It also prints
> '0x2 ^ NO_RESIZING' as an alternative to silence the warning. Given
> the
> comment just above the changed line, I think the 0x2 version is
> better
> than what I have in the patch now, since it still uses NO_RESIZING.
> But
> I'm open for suggestions on how to keep the code as clear as
> possible.

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.

The clang warning alternative is also slightly confusing. I think what
it means to say is that it sees CLEANING is a constant int not
expressed as a hex value. It really should show the definition:
#define CLEANING 2u.

As far as I understand, if it sees a ^ xor on a hex value it knows you
are dealing with bits, but if it sees it on a non-hex constant it isn't
sure you are serious about xoring.

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
 
 #define STATE_BITS 2u
 #define STATE_INCREMENT (1u << STATE_BITS)
@@ -284,7 +284,7 @@ resize_master(NAME *htab)
 
   free(htab->old_table);
 
-  /* Change state to NO_RESIZING */
+  /* Change state from CLEANING to NO_RESIZING */
   atomic_fetch_xor_explicit(&htab->resizing_state, CLEANING ^ NO_RESIZING,
                             memory_order_relaxed);