On 2023-03-21 06:15, Ondřej Surý wrote:

On 20. 3. 2023, at 19:31, Mathieu Desnoyers <mathieu.desnoy...@efficios.com> 
wrote:

On 2023-03-17 17:37, Ondřej Surý via lttng-dev wrote:
When adding REMOVED_FLAG to the pointers in the rculfhash
implementation, retype the generic pointer to uintptr_t to fix the
compiler error.

What is the compiler error ? I'm wondering whether the expected choice
to match the rest of this file's content would be to use "uintptr_t *" or "unsigned 
long *" ?

This is the error:

rculfhash.c:1201:2: error: address argument to atomic operation must be a 
pointer to integer ('struct cds_lfht_node **' invalid)
         uatomic_or(&node->next, REMOVED_FLAG);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/urcu/uatomic.h:60:8: note: expanded from macro 'uatomic_or'
         (void)__atomic_or_fetch((addr), (mask), __ATOMIC_RELAXED)
               ^                 ~~~~~~
rculfhash.c:1444:3: error: address argument to atomic operation must be a 
pointer to integer ('struct cds_lfht_node **' invalid)
                 uatomic_or(&fini_bucket->next, REMOVED_FLAG);
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/urcu/uatomic.h:60:8: note: expanded from macro 'uatomic_or'
         (void)__atomic_or_fetch((addr), (mask), __ATOMIC_RELAXED)
               ^                 ~~~~~~

uintptr_t is defined as "unsigned integer type capable of holding a pointer to 
void" while unsigned long is at least 32-bit;

I guess that works in a practise, but using unsigned long to retype the 
pointers might blow up (thinking of x32 which I know
little about, but it's kind of hybrid architecture, isn't it?)

x32 uses 4 bytes for unsigned long, uintptr_t, and void * size. So even that architecture is OK with casting pointer to unsigned long.

I agree with you that uintptr_t is the semantically correct type, but it should come as a separate change across the urcu code base: currently there are many places where void * is cast to unsigned long to do bitwise operations.

I therefore recommend to use unsigned long here to stay similar to the rest of the code base, and keep the transition from unsigned long to uintptr_t for the future, as it is not an immediate issue we have to address.

Thanks,

Mathieu



Ondrej
--
Ondřej Surý (He/Him)
ond...@sury.org


--
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Reply via email to