Remove misleading XOR

Timm Bäder tbaeder@redhat.com
Thu Feb 18 07:41:01 GMT 2021


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



More information about the Elfutils-devel mailing list