Re: [PATCH v2] sysdeps/nanosleep: Use clock_nanosleep_time64 if avaliable

[Alistair Francis <alistair23 at gmail dot com>]

On Mon, Nov 4, 2019 at 11:30 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 04/11/2019 15:16, Alistair Francis wrote:
> > On Tue, Oct 29, 2019 at 1:16 PM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >>
> >>
> >> On 24/10/2019 20:41, Alistair Francis wrote:
>
> >>
> >> There is no need to replicate all the syscall logic, nanosleep can be implemented
> >> with __clock_nanosleep.  You can do:
> >>
> >> int
> >> __nanosleep (const struct timespec *requested_time,
> >>              struct timespec *remaining)
> >> {
> >>   int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);
> >>   if (ret != 0)
> >>     {
> >>       __set_errno (-ret);
> >>       return -1;
> >>     }
> >>   return ret;
> >> }
> >
> > This doesn't work as __clock_nanosleep() isn't avaliable in nptl so it
> > fails to compile. My v3 patch attempted to fix this, but that also
> > doesn't work and ends up squishing some patches together.
>
> For this case you will need to add __clock_nanosleep as a GLIBC_PRIVATE
> exported symbol from libc and add a hidden_proto/hidden_def for libc
> internal plt avoidance (as below).

Thanks for explaining this.

>
> However, I agree with Joseph and Florian that for this case it would be
> better to move the symbol from libpthread to libc. I will send a patch
> to refactor it.

I am going to include the diff below and convert __nanosleep to call
__clock_nanosleep(), but I won't change the file structure or move
anything else around. I'll leave that to you in a follow up patch.

Alistair

>
> diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions
> index d385085c61..475abb5004 100644
> --- a/sysdeps/unix/sysv/linux/Versions
> +++ b/sysdeps/unix/sysv/linux/Versions
> @@ -187,5 +187,6 @@ libc {
>      __sigtimedwait;
>      # functions used by nscd
>      __netlink_assert_response;
> +    __clock_nanosleep;
>    }
>  }
> diff --git a/sysdeps/unix/sysv/linux/clock_nanosleep.c b/sysdeps/unix/sysv/linux/clock_nanosleep.c
> index 1f240b8720..f3c6fd2d5f 100644
> --- a/sysdeps/unix/sysv/linux/clock_nanosleep.c
> +++ b/sysdeps/unix/sysv/linux/clock_nanosleep.c
> @@ -42,7 +42,7 @@ __clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,
>    return (INTERNAL_SYSCALL_ERROR_P (r, err)
>           ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);
>  }
> -
> +libc_hidden_def (__clock_nanosleep)
>  versioned_symbol (libc, __clock_nanosleep, clock_nanosleep, GLIBC_2_17);
>  /* clock_nanosleep moved to libc in version 2.17;
>     old binaries may expect the symbol version it had in librt.  */
> diff --git a/sysdeps/unix/sysv/linux/nanosleep.c b/sysdeps/unix/sysv/linux/nanosleep.c
> index 6787909248..1e2481847b 100644
> --- a/sysdeps/unix/sysv/linux/nanosleep.c
> +++ b/sysdeps/unix/sysv/linux/nanosleep.c
> @@ -25,7 +25,14 @@ int
>  __nanosleep (const struct timespec *requested_time,
>              struct timespec *remaining)
>  {
> -  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);
> +  //return SYSCALL_CANCEL (nanosleep, requested_time, remaining);
> +  int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);
> +  if (ret != 0)
> +    {
> +      __set_errno (-ret);
> +      return -1;
> +    }
> +  return ret;
>  }
>  hidden_def (__nanosleep)
>  weak_alias (__nanosleep, nanosleep)