On Sat, 1 Mar 2025 at 15:17, Alejandro Colomar <alx@xxxxxxxxxx> wrote: > > Hi Jonathan, rahl, > > On Sat, Mar 01, 2025 at 03:08:50PM +0000, Jonathan Wakely wrote: > > > >BTW, there seem to be other important bugs in that example program, > > > >which I don't understand either. Would you mind having a look at those > > > >(I'm assuming that you seem familiar with these atomic APIs)? See: > > > > > > You're right, there are more bugs. I'm however not so familiar with these functions, but I do have some help. > > > > > > The errors in question relate to a missing _Atomic qualifier for several variables and function parameters. > > > The documentation linked above should clarify this as well. > > > > > > It turns out these calls could be replaced with a compiler built-in (__atomic_compare_exchange_n) both for clang and gcc, which wouldn't have the above problem, and would also allow for the removal of 'stdatomic.h'. However, this didn't feel too in keeping with manpage example code. > > > > Well the example already relies on Linux-specific details of > > syscall(2) and futex(2), and only GCC-compatible compilers are really > > usable on Linux. So the GCC extensions could probably be assumed to > > exist. > > > > If you were writing pure C11 code intended to be portable, you > > wouldn't be using SYS_futex anyway. > > *Iff* C11 atomics were something we understand, maybe even just having > a working example that uses it would be worth it, even if Linux-only > stuff can do the same. I understand the C11 atomics fine, I don't understand how they interact with syscall(2). > > However... > > > > The gcc docs are here: > > > <https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html> > > > > The patch assumes that you can pass _Atomic int* to the futex syscall, > > but I'm not sure if that's correct. The syscall expects an int. > > Hmmm. > > > > > The C standard says "NOTE The representation of atomic integer types > > need not have the same size as their corresponding regular types." > > > > With GCC and Clang, _Atomic uint32_t does have the same representation > > as unsigned, so it should work. I'm not an expert here though. > > Considering that the example has been broken for a long time, and I've > asked for help in the glibc-help@ mailing list and nobody answered, and > you don't feel comfortable with it either, the best course of action is > to revert that commit. The __sync_xxx built-ins are documented as legacy and not to be used in new code: https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html So while reverting it seems better than having broken examples, it would be better to not use the legacy APIs.
