On Sat, 1 Mar 2025 at 11:36, rahl <rahl@xxxxxxxxxx> wrote: > > On 28 February 2025 20:57:50 UTC, Alejandro Colomar <alx@xxxxxxxxxx> wrote: > >On Fri, Feb 28, 2025 at 10:13:04AM +0000, rahl wrote: > >> I noticed that 'const' is used for both 'one' and 'zero' variables in > >> the Examples section demo code of manpage 'futex(2)'. > >> > >> The variables are both used in calls to > >> 'atomic_compare_exchange_strong()' where the 'const' is discarded as > >> it may write to the 'expected' parameter during a "failure" case. > > > >I don't understand what that function is. It doesn't have a manual > >page, and it's neither in /usr/include. It doesn't appear in the GCC > >manual either. And it's not described in ISO C. > > > >What is that function? > > > >I'd like to understand what we're calling to be able to understand how > >the calling code is wrong. > > atomic_compare_exchange_XXX() are defined in stdatomic.h and were introduced in C11 (ISO/IEC 9899:2011 I believe). > > The main online documentation for these that I'm currently aware of is at cppreference. > <https://en.cppreference.com/w/c/atomic/atomic_compare_exchange> > > >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 futex.2 examples used to use the non-standard __sync_bool_compare_and_swap, which was correct. It was changed to use atomic_compare_exchange_strong by: commit 915c4ba36f9f71db88e7e7913b845d046996f485 Author: Benjamin Peterson <benjamin@xxxxxxxxxx> Date: Tue Nov 13 22:53:41 2018 -0800 futex.2: Make the example use C11 atomics rather than GCC builtins That change was wrong and broke the examples, because C11 atomics are only usable on _Atomic variables. __sync_bool_compare_and_swap and __atomic_compare_exchange are usable with plain non-_Atomic types. > 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. > > The gcc docs are here: > <https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html> > > >> Attached is the trivial patch to remove the offending qualifiers. > > > >Please provide a complete patch, with a commit message. See the files > >under <CONTRIBUTING.d/>. > > Rather than submit a separate email, I have attached a file containing the full output from git-format-patch, with the message being in what I believe is the expected format - having read that an attachment is also an acceptable approach, albeit perhaps not in email form. > If for some reason you feel it should be submitted as a separate email I can do so. > This way seemed more prudent for now as this is my first such patch and I'd like to get it right before having multiple threads in the ML. > > Let me know if there are any further issues. > Cheers, > > rahl
