Re: [PATCH 2/2] futex.2: Use appropriate types

[Alejandro Colomar <colomar.6.4.3@xxxxxxxxx>]

BTW, apparently the kernel doesn't use 'const' for 'utime'
('timeout' in the manual page),
but effectively, it doesn't modify it, AFAICS.

Should the kernel use 'const'?
Is there a reason for the kernel not using 'const'?
Should we do anything about it in the manual page?

Thanks,

Alex

On 2020-10-30 13:39, Alejandro Colomar wrote:
> The Linux kernel uses the following:
>
> kernel/futex.c:3778:
> SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
> 		struct __kernel_timespec __user *, utime, u32 __user *, uaddr2,
> 		u32, val3)
>
> Since there is no glibc wrapper, use the same types the kernel uses.
>
> Signed-off-by: Alejandro Colomar <colomar.6.4.3@xxxxxxxxx>
> ---
>   man2/futex.2 | 27 ++++++++++++++-------------
>   1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/man2/futex.2 b/man2/futex.2
> index 837adbd25..73de71623 100644
> --- a/man2/futex.2
> +++ b/man2/futex.2
> @@ -26,12 +26,13 @@ futex \- fast user-space locking
>   .nf
>   .PP
>   .B #include <linux/futex.h>
> +.B #include <stdint.h>
>   .B #include <sys/time.h>
>   .PP
> -.BI "int futex(int *" uaddr ", int " futex_op ", int " val ,
> +.BI "long futex(uint32_t *" uaddr ", int " futex_op ", uint32_t " val ,
>   .BI "          const struct timespec *" timeout , \
>   " \fR  /* or: \fBuint32_t \fIval2\fP */"
> -.BI "          int *" uaddr2 ", int " val3 );
> +.BI "          uint32_t *" uaddr2 ", uint32_t " val3 );
>   .fi
>   .PP
>   .IR Note :
> @@ -581,8 +582,8 @@ any of the two supplied futex words:
>   .IP
>   .in +4n
>   .EX
> -int oldval = *(int *) uaddr2;
> -*(int *) uaddr2 = oldval \fIop\fP \fIoparg\fP;
> +uint32_t oldval = *(uint32_t *) uaddr2;
> +*(uint32_t *) uaddr2 = oldval \fIop\fP \fIoparg\fP;
>   futex(uaddr, FUTEX_WAKE, val, 0, 0, 0);
>   if (oldval \fIcmp\fP \fIcmparg\fP)
>       futex(uaddr2, FUTEX_WAKE, val2, 0, 0, 0);
> @@ -1765,11 +1766,11 @@ Child  (18535) 4
>   #define errExit(msg)    do { perror(msg); exit(EXIT_FAILURE); \e
>                           } while (0)
>   
> -static int *futex1, *futex2, *iaddr;
> +static uint32_t *futex1, *futex2, *iaddr;
>   
>   static int
> -futex(int *uaddr, int futex_op, int val,
> -      const struct timespec *timeout, int *uaddr2, int val3)
> +futex(uint32_t *uaddr, int futex_op, uint32_t val,
> +      const struct timespec *timeout, uint32_t *uaddr2, uint32_t val3)
>   {
>       return syscall(SYS_futex, uaddr, futex_op, val,
>                      timeout, uaddr2, val3);
> @@ -1779,9 +1780,9 @@ futex(int *uaddr, int futex_op, int val,
>      become 1, and then set the value to 0. */
>   
>   static void
> -fwait(int *futexp)
> +fwait(uint32_t *futexp)
>   {
> -    int s;
> +    long s;
>   
>       /* atomic_compare_exchange_strong(ptr, oldval, newval)
>          atomically performs the equivalent of:
> @@ -1794,7 +1795,7 @@ fwait(int *futexp)
>       while (1) {
>   
>           /* Is the futex available? */
> -        const int one = 1;
> +        const uint32_t one = 1;
>           if (atomic_compare_exchange_strong(futexp, &one, 0))
>               break;      /* Yes */
>   
> @@ -1811,13 +1812,13 @@ fwait(int *futexp)
>      so that if the peer is blocked in fpost(), it can proceed. */
>   
>   static void
> -fpost(int *futexp)
> +fpost(uint32_t *futexp)
>   {
> -    int s;
> +    long s;
>   
>       /* atomic_compare_exchange_strong() was described in comments above */
>   
> -    const int zero = 0;
> +    const uint32_t zero = 0;
>       if (atomic_compare_exchange_strong(futexp, &zero, 1)) {
>           s = futex(futexp, FUTEX_WAKE, 1, NULL, NULL, 0);
>           if (s  == \-1)