public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX
@ 2020-10-16 10:09 Chen Li
  2020-10-26 19:44 ` Adhemerval Zanella
  0 siblings, 1 reply; 20+ messages in thread
From: Chen Li @ 2020-10-16 10:09 UTC (permalink / raw)
  To: libc-alpha


According to linux's manpage and posix's doc, errno should be
set to ENAMETOOLONG if the path exceeds the maximuz length:

linux man page(http://man7.org/linux/man-pages/man3/shm_open.3.html)

```
ENAMETOOLONG
   The length of name exceeds PATH_MAX.
```

posix doc(https://pubs.opengroup.org/onlinepubs/009695399/functions/shm_open.html):

```
[ENAMETOOLONG]
   The length of the name argument exceeds {PATH_MAX} or a pathname component is longer than {NAME_MAX}.
```
glibc doesn't handle ENAMETOOLONG correctly previously. When the path
exceeds the maximum value, errno was set to EINVAL instead, which
doesn't conform the man page and posix standard.

This patch removes the NAME_MAX check in SHM_GET_NAME and leaves this
check to open syscall, which should handle maximunize length correctly
inside various filesystem implementations.
---
 sysdeps/posix/shm-directory.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sysdeps/posix/shm-directory.h b/sysdeps/posix/shm-directory.h
index c7979ebb72..5a1aab2c14 100644
--- a/sysdeps/posix/shm-directory.h
+++ b/sysdeps/posix/shm-directory.h
@@ -53,7 +53,7 @@ extern const char *__shm_directory (size_t *len);
     ++name;								      \
   size_t namelen = strlen (name) + 1;					      \
   /* Validate the filename.  */						      \
-  if (namelen == 1 || namelen >= NAME_MAX || strchr (name, '/') != NULL)      \
+  if (namelen == 1 || strchr (name, '/') != NULL)      \
     {									      \
       __set_errno (errno_for_invalid);					      \
       return retval_for_invalid;					      \



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX
  2020-10-16 10:09 [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX Chen Li
@ 2020-10-26 19:44 ` Adhemerval Zanella
  2020-10-27 10:27   ` Florian Weimer
  0 siblings, 1 reply; 20+ messages in thread
From: Adhemerval Zanella @ 2020-10-26 19:44 UTC (permalink / raw)
  To: libc-alpha



On 16/10/2020 07:09, Chen Li wrote:
> 
> According to linux's manpage and posix's doc, errno should be
> set to ENAMETOOLONG if the path exceeds the maximuz length:
> 
> linux man page(http://man7.org/linux/man-pages/man3/shm_open.3.html)
> 
> ```
> ENAMETOOLONG
>    The length of name exceeds PATH_MAX.
> ```
> 
> posix doc(https://pubs.opengroup.org/onlinepubs/009695399/functions/shm_open.html):
> 
> ```
> [ENAMETOOLONG]
>    The length of the name argument exceeds {PATH_MAX} or a pathname component is longer than {NAME_MAX}.
> ```
> glibc doesn't handle ENAMETOOLONG correctly previously. When the path
> exceeds the maximum value, errno was set to EINVAL instead, which
> doesn't conform the man page and posix standard.
> 
> This patch removes the NAME_MAX check in SHM_GET_NAME and leaves this
> check to open syscall, which should handle maximunize length correctly
> inside various filesystem implementations.

Although it fixes the errno value for large filenames, it also allows a
possible unbounded stack allocation since the resulting path will be
issued with alloca.

I think it would be good to refactor the code to use a PATH_MAX variable
instead, something like:

  _Bool
  shm_get_name (const char *prefix, char *shm_name, size shm_path_max)
  {
    size_t shm_dirlen;
    const char *shm_dir = __shm_directory (&shm_dirlen);
    if (shm_dir == NULL)
      {
        __set_errno (ENOSYS);
        return false;
      }
    while (name[0] == '/')
      ++name;
    size_t namelen = strlen (name) + 1;
    if (namelen == 1 || strchr (name, '/') != NULL)
      {
        __set_errno (EINVAL);
        result false;
      }
    if (shm_dirlen + namelen > shm_path_max)
      {
        __set_errno (ENAMETOOLONG);
        result false;
      }
    __mempcpy (__memcpy (shm_name, shm_dir, shm_dirlen),
               name, namelen);
    return true;
  }

It would be good to move shm_get_name to its own implementation file as
well (since it is used on both shm_open and shm_unlink).

So on sysdeps/posix/shm_open.c:

  int
  shm_open (const char *name, int oflag, mode_t mode)
  {
    char shm_path[PATH_MAX];
    if (! shm_get_name (shm_path, sizeof shm_path))
      return SEM_FAILED;
    [...]
  }

> ---
>  sysdeps/posix/shm-directory.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sysdeps/posix/shm-directory.h b/sysdeps/posix/shm-directory.h
> index c7979ebb72..5a1aab2c14 100644
> --- a/sysdeps/posix/shm-directory.h
> +++ b/sysdeps/posix/shm-directory.h
> @@ -53,7 +53,7 @@ extern const char *__shm_directory (size_t *len);
>      ++name;								      \
>    size_t namelen = strlen (name) + 1;					      \
>    /* Validate the filename.  */						      \
> -  if (namelen == 1 || namelen >= NAME_MAX || strchr (name, '/') != NULL)      \
> +  if (namelen == 1 || strchr (name, '/') != NULL)      \
>      {									      \
>        __set_errno (errno_for_invalid);					      \
>        return retval_for_invalid;					      \
> 
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX
  2020-10-26 19:44 ` Adhemerval Zanella
@ 2020-10-27 10:27   ` Florian Weimer
  2020-10-27 11:53     ` Adhemerval Zanella
  0 siblings, 1 reply; 20+ messages in thread
From: Florian Weimer @ 2020-10-27 10:27 UTC (permalink / raw)
  To: Adhemerval Zanella via Libc-alpha

* Adhemerval Zanella via Libc-alpha:

> I think it would be good to refactor the code to use a PATH_MAX variable
> instead, something like:

As this is generic code, I don't think we can assume PATH_MAX is defined
(Hurd does not have it AFAIK).

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX
  2020-10-27 10:27   ` Florian Weimer
@ 2020-10-27 11:53     ` Adhemerval Zanella
  0 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2020-10-27 11:53 UTC (permalink / raw)
  To: Florian Weimer, Adhemerval Zanella via Libc-alpha



On 27/10/2020 07:27, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> I think it would be good to refactor the code to use a PATH_MAX variable
>> instead, something like:
> 
> As this is generic code, I don't think we can assume PATH_MAX is defined
> (Hurd does not have it AFAIK).

Although Hurd uses the POSIX implementation, I am not sure if it is fully
functional.  It does not support sem [1] [2].

I would like to avoid creating a unbounded path, at least on Linux with
BZ#25383 expected fix it would highly unlikely we will even need paths
larger than PATH_MAX (and current implementation does imposes the NAME_MAX
limit).

So I think it would be better to make Linux uses at maximum PATH_MAX, or 
even NAME_MAX plus "/dev/shm/".

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=25524
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=25521

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/3] y2038: Convert cnd_timedwait to support 64 bit time
@ 2020-11-03  9:14 Lukasz Majewski
  2020-11-03  9:14 ` [PATCH v2 2/3] y2038: Convert mtx_timedlock " Lukasz Majewski
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Lukasz Majewski @ 2020-11-03  9:14 UTC (permalink / raw)
  To: Joseph Myers, Paul Eggert, Adhemerval Zanella
  Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
	Florian Weimer, Carlos O'Donell, Stepan Golosunov,
	Andreas Schwab, Zack Weinberg, Lukasz Majewski

The cnd_timedwait function has been converted to support 64 bit time.
It was also necessary to provide Linux specific copy of it to avoid
problems on i686-gnu (i.e. HURD) port, which is not providing
pthread_cond_timedwait() supporting 64 bit time.

The cnd_timedwait is a wrapper on POSIX threads to provide C11 standard
threads interface. It directly calls __pthread_cond_timedwait64().

Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
 sysdeps/pthread/thrd_priv.h             |  8 +++++
 sysdeps/unix/sysv/linux/cnd_timedwait.c | 44 +++++++++++++++++++++++++
 2 files changed, 52 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/cnd_timedwait.c

diff --git a/sysdeps/pthread/thrd_priv.h b/sysdeps/pthread/thrd_priv.h
index d22ad6f632..dbfec0df7a 100644
--- a/sysdeps/pthread/thrd_priv.h
+++ b/sysdeps/pthread/thrd_priv.h
@@ -24,6 +24,14 @@
 #include <errno.h>
 #include "pthreadP.h"	/* For pthread_{mutex,cond}_t definitions.  */
 
+#if __TIMESIZE == 64
+# define __cnd_timedwait64 __cnd_timedwait
+#else
+extern int __cnd_timedwait64 (cnd_t *restrict cond, mtx_t *restrict mutex,
+                              const struct __timespec64* restrict time_point);
+libpthread_hidden_proto (__cnd_timedwait64)
+#endif
+
 static __always_inline int
 thrd_err_map (int err_code)
 {
diff --git a/sysdeps/unix/sysv/linux/cnd_timedwait.c b/sysdeps/unix/sysv/linux/cnd_timedwait.c
new file mode 100644
index 0000000000..5bf5a2d968
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/cnd_timedwait.c
@@ -0,0 +1,44 @@
+/* C11 threads conditional timed wait implementation - Linux variant.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <time.h>
+#include "thrd_priv.h"
+
+int
+__cnd_timedwait64 (cnd_t *restrict cond, mtx_t *restrict mutex,
+                   const struct __timespec64* restrict time_point)
+{
+  int err_code = __pthread_cond_timedwait64 ((pthread_cond_t *) cond,
+                                             (pthread_mutex_t *) mutex,
+                                             time_point);
+  return thrd_err_map (err_code);
+}
+
+#if __TIMESIZE != 64
+libpthread_hidden_def (__cnd_timedwait64)
+
+int
+__cnd_timedwait (cnd_t *restrict cond, mtx_t *restrict mutex,
+                 const struct timespec* restrict time_point)
+{
+  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*time_point);
+
+  return __cnd_timedwait64(cond, mutex, &ts64);
+}
+#endif
+weak_alias (__cnd_timedwait, cnd_timedwait)
-- 
2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 2/3] y2038: Convert mtx_timedlock to support 64 bit time
  2020-11-03  9:14 [PATCH v2 1/3] y2038: Convert cnd_timedwait to support 64 bit time Lukasz Majewski
@ 2020-11-03  9:14 ` Lukasz Majewski
  2020-11-03 14:45   ` [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX Chen Li
                     ` (2 more replies)
  2020-11-03  9:14 ` [PATCH v2 3/3] y2038: Convert thrd_sleep " Lukasz Majewski
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 20+ messages in thread
From: Lukasz Majewski @ 2020-11-03  9:14 UTC (permalink / raw)
  To: Joseph Myers, Paul Eggert, Adhemerval Zanella
  Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
	Florian Weimer, Carlos O'Donell, Stepan Golosunov,
	Andreas Schwab, Zack Weinberg, Lukasz Majewski

The mtx_timedlock function has been converted to support 64 bit time.
It was also necessary to provide Linux specific copy of it to avoid
problems on i686-gnu (i.e. HURD) port, which is not providing
pthread_mutex_timedlock() supporting 64 bit time.

The mtx_timedlock is a wrapper on POSIX threads to provide C11 standard
threads interface. It directly calls __pthread_mutex_timedlock64().

Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
 sysdeps/pthread/thrd_priv.h             |  4 +++
 sysdeps/unix/sysv/linux/mtx_timedlock.c | 43 +++++++++++++++++++++++++
 2 files changed, 47 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/mtx_timedlock.c

diff --git a/sysdeps/pthread/thrd_priv.h b/sysdeps/pthread/thrd_priv.h
index dbfec0df7a..efe453bdec 100644
--- a/sysdeps/pthread/thrd_priv.h
+++ b/sysdeps/pthread/thrd_priv.h
@@ -26,10 +26,14 @@
 
 #if __TIMESIZE == 64
 # define __cnd_timedwait64 __cnd_timedwait
+# define __mtx_timedlock64 __mtx_timedlock
 #else
 extern int __cnd_timedwait64 (cnd_t *restrict cond, mtx_t *restrict mutex,
                               const struct __timespec64* restrict time_point);
 libpthread_hidden_proto (__cnd_timedwait64)
+extern int __mtx_timedlock64 (mtx_t *restrict mutex,
+                              const struct __timespec64 *restrict time_point);
+libpthread_hidden_proto (__mtx_timedlock64)
 #endif
 
 static __always_inline int
diff --git a/sysdeps/unix/sysv/linux/mtx_timedlock.c b/sysdeps/unix/sysv/linux/mtx_timedlock.c
new file mode 100644
index 0000000000..ef57dd1e41
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mtx_timedlock.c
@@ -0,0 +1,43 @@
+/* C11 threads mutex timed lock implementation - Linux variant.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <time.h>
+#include "thrd_priv.h"
+
+int
+__mtx_timedlock64 (mtx_t *restrict mutex,
+                   const struct __timespec64 *restrict time_point)
+{
+  int err_code = __pthread_mutex_timedlock64 ((pthread_mutex_t *)mutex,
+                                              time_point);
+  return thrd_err_map (err_code);
+}
+
+#if __TIMESIZE != 64
+libpthread_hidden_def (__mtx_timedlock64)
+
+int
+__mtx_timedlock (mtx_t *restrict mutex,
+                 const struct timespec *restrict time_point)
+{
+  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*time_point);
+
+  return __mtx_timedlock64(mutex, &ts64);
+}
+#endif
+weak_alias (__mtx_timedlock, mtx_timedlock)
-- 
2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 3/3] y2038: Convert thrd_sleep to support 64 bit time
  2020-11-03  9:14 [PATCH v2 1/3] y2038: Convert cnd_timedwait to support 64 bit time Lukasz Majewski
  2020-11-03  9:14 ` [PATCH v2 2/3] y2038: Convert mtx_timedlock " Lukasz Majewski
@ 2020-11-03  9:14 ` Lukasz Majewski
  2020-11-03 15:32   ` Alistair Francis
  2020-11-11 20:08   ` Adhemerval Zanella
  2020-11-03 15:24 ` [PATCH v2 1/3] y2038: Convert cnd_timedwait " Alistair Francis
  2020-11-11 20:05 ` Adhemerval Zanella
  3 siblings, 2 replies; 20+ messages in thread
From: Lukasz Majewski @ 2020-11-03  9:14 UTC (permalink / raw)
  To: Joseph Myers, Paul Eggert, Adhemerval Zanella
  Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
	Florian Weimer, Carlos O'Donell, Stepan Golosunov,
	Andreas Schwab, Zack Weinberg, Lukasz Majewski

The thrd_sleep function has been converted to support 64 bit time.
It was also necessary to provide Linux specific copy of it to avoid
problems on i686-gnu (i.e. HURD) port, which is not providing
clock_nanosleep() supporting 64 bit time.

The thrd_sleep is a wrapper on POSIX threads to provide C11 standard
threads interface. It directly calls __clock_nanosleep64().

Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
 sysdeps/pthread/thrd_priv.h          |  4 ++
 sysdeps/unix/sysv/linux/thrd_sleep.c | 55 ++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/thrd_sleep.c

diff --git a/sysdeps/pthread/thrd_priv.h b/sysdeps/pthread/thrd_priv.h
index efe453bdec..c53d47a02c 100644
--- a/sysdeps/pthread/thrd_priv.h
+++ b/sysdeps/pthread/thrd_priv.h
@@ -27,6 +27,7 @@
 #if __TIMESIZE == 64
 # define __cnd_timedwait64 __cnd_timedwait
 # define __mtx_timedlock64 __mtx_timedlock
+# define __thrd_sleep64 __thrd_sleep
 #else
 extern int __cnd_timedwait64 (cnd_t *restrict cond, mtx_t *restrict mutex,
                               const struct __timespec64* restrict time_point);
@@ -34,6 +35,9 @@ libpthread_hidden_proto (__cnd_timedwait64)
 extern int __mtx_timedlock64 (mtx_t *restrict mutex,
                               const struct __timespec64 *restrict time_point);
 libpthread_hidden_proto (__mtx_timedlock64)
+extern int __thrd_sleep64 (const struct __timespec64* time_point,
+                           struct __timespec64* remaining);
+libpthread_hidden_proto (__thrd_sleep64)
 #endif
 
 static __always_inline int
diff --git a/sysdeps/unix/sysv/linux/thrd_sleep.c b/sysdeps/unix/sysv/linux/thrd_sleep.c
new file mode 100644
index 0000000000..632527d26a
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/thrd_sleep.c
@@ -0,0 +1,55 @@
+/* C11 threads thread sleep implementation - Linux variant.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <time.h>
+#include <sysdep-cancel.h>
+
+#include "thrd_priv.h"
+
+int
+__thrd_sleep64 (const struct __timespec64* time_point,
+                struct __timespec64* remaining)
+{
+  int ret = __clock_nanosleep_time64 (CLOCK_REALTIME, 0, time_point, remaining);
+  /* C11 states thrd_sleep function returns -1 if it has been interrupted
+     by a signal, or a negative value if it fails.  */
+  switch (ret)
+  {
+     case 0:      return 0;
+     case EINTR:  return -1;
+     default:     return -2;
+  }
+}
+
+#if __TIMESIZE != 64
+libpthread_hidden_def (__thrd_sleep64)
+
+int
+__thrd_sleep (const struct timespec* time_point, struct timespec* remaining)
+{
+  const struct __timespec64 treq64 = valid_timespec_to_timespec64 (*time_point);
+  struct __timespec64 trem64;
+
+  int ret = __thrd_sleep64 (&treq64, &trem64);
+  if (ret == -1 && remaining != NULL)
+    *remaining = valid_timespec64_to_timespec (trem64);
+
+  return ret;
+}
+#endif
+weak_alias (__thrd_sleep, thrd_sleep)
-- 
2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX
  2020-11-03  9:14 ` [PATCH v2 2/3] y2038: Convert mtx_timedlock " Lukasz Majewski
@ 2020-11-03 14:45   ` Chen Li
  2020-11-03 16:43     ` Adhemerval Zanella
  2020-11-03 15:28   ` [PATCH v2 2/3] y2038: Convert mtx_timedlock to support 64 bit time Alistair Francis
  2020-11-11 20:06   ` Adhemerval Zanella
  2 siblings, 1 reply; 20+ messages in thread
From: Chen Li @ 2020-11-03 14:45 UTC (permalink / raw)
  To: Adhemerval Zanella, GNU C Library


On  Mon Oct 26 19:44:44 GMT 2020,
Adhemerval Zanella  wrote:

> I think it would be good to refactor the code to use a PATH_MAX variable
> instead, something like:
>
>  _Bool
> shm_get_name (const char *prefix, char *shm_name, size shm_path_max)

So you prefer function over macro here? If so, shm_dir, shm_dirlen, shm_name
should all be passed as args too, because unlike c macro, function is not
dynamical scope. Given such long arg list(errno_for_invalid, retval_for_invalid,
prefix, shm_dir, shm_dirlen, shm_name), I'm not sure weathre it confrom glibc's
coding style, and is refactoring to split into more small functions a good idea? 


Thank,
Chen Li



^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/3] y2038: Convert cnd_timedwait to support 64 bit time
  2020-11-03  9:14 [PATCH v2 1/3] y2038: Convert cnd_timedwait to support 64 bit time Lukasz Majewski
  2020-11-03  9:14 ` [PATCH v2 2/3] y2038: Convert mtx_timedlock " Lukasz Majewski
  2020-11-03  9:14 ` [PATCH v2 3/3] y2038: Convert thrd_sleep " Lukasz Majewski
@ 2020-11-03 15:24 ` Alistair Francis
  2020-11-11 20:05 ` Adhemerval Zanella
  3 siblings, 0 replies; 20+ messages in thread
From: Alistair Francis @ 2020-11-03 15:24 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Joseph Myers, Paul Eggert, Adhemerval Zanella, Arnd Bergmann,
	Alistair Francis, GNU C Library, Florian Weimer,
	Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
	Zack Weinberg

On Tue, Nov 3, 2020 at 1:15 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> The cnd_timedwait function has been converted to support 64 bit time.
> It was also necessary to provide Linux specific copy of it to avoid
> problems on i686-gnu (i.e. HURD) port, which is not providing
> pthread_cond_timedwait() supporting 64 bit time.
>
> The cnd_timedwait is a wrapper on POSIX threads to provide C11 standard
> threads interface. It directly calls __pthread_cond_timedwait64().
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  sysdeps/pthread/thrd_priv.h             |  8 +++++
>  sysdeps/unix/sysv/linux/cnd_timedwait.c | 44 +++++++++++++++++++++++++
>  2 files changed, 52 insertions(+)
>  create mode 100644 sysdeps/unix/sysv/linux/cnd_timedwait.c
>
> diff --git a/sysdeps/pthread/thrd_priv.h b/sysdeps/pthread/thrd_priv.h
> index d22ad6f632..dbfec0df7a 100644
> --- a/sysdeps/pthread/thrd_priv.h
> +++ b/sysdeps/pthread/thrd_priv.h
> @@ -24,6 +24,14 @@
>  #include <errno.h>
>  #include "pthreadP.h"  /* For pthread_{mutex,cond}_t definitions.  */
>
> +#if __TIMESIZE == 64
> +# define __cnd_timedwait64 __cnd_timedwait
> +#else
> +extern int __cnd_timedwait64 (cnd_t *restrict cond, mtx_t *restrict mutex,
> +                              const struct __timespec64* restrict time_point);
> +libpthread_hidden_proto (__cnd_timedwait64)
> +#endif
> +
>  static __always_inline int
>  thrd_err_map (int err_code)
>  {
> diff --git a/sysdeps/unix/sysv/linux/cnd_timedwait.c b/sysdeps/unix/sysv/linux/cnd_timedwait.c
> new file mode 100644
> index 0000000000..5bf5a2d968
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/cnd_timedwait.c
> @@ -0,0 +1,44 @@
> +/* C11 threads conditional timed wait implementation - Linux variant.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <time.h>
> +#include "thrd_priv.h"
> +
> +int
> +__cnd_timedwait64 (cnd_t *restrict cond, mtx_t *restrict mutex,
> +                   const struct __timespec64* restrict time_point)
> +{
> +  int err_code = __pthread_cond_timedwait64 ((pthread_cond_t *) cond,
> +                                             (pthread_mutex_t *) mutex,
> +                                             time_point);
> +  return thrd_err_map (err_code);
> +}
> +
> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__cnd_timedwait64)
> +
> +int
> +__cnd_timedwait (cnd_t *restrict cond, mtx_t *restrict mutex,
> +                 const struct timespec* restrict time_point)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*time_point);
> +
> +  return __cnd_timedwait64(cond, mutex, &ts64);
> +}
> +#endif
> +weak_alias (__cnd_timedwait, cnd_timedwait)
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/3] y2038: Convert mtx_timedlock to support 64 bit time
  2020-11-03  9:14 ` [PATCH v2 2/3] y2038: Convert mtx_timedlock " Lukasz Majewski
  2020-11-03 14:45   ` [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX Chen Li
@ 2020-11-03 15:28   ` Alistair Francis
  2020-11-11 20:06   ` Adhemerval Zanella
  2 siblings, 0 replies; 20+ messages in thread
From: Alistair Francis @ 2020-11-03 15:28 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Joseph Myers, Paul Eggert, Adhemerval Zanella, Arnd Bergmann,
	Alistair Francis, GNU C Library, Florian Weimer,
	Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
	Zack Weinberg

On Tue, Nov 3, 2020 at 1:15 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> The mtx_timedlock function has been converted to support 64 bit time.
> It was also necessary to provide Linux specific copy of it to avoid
> problems on i686-gnu (i.e. HURD) port, which is not providing
> pthread_mutex_timedlock() supporting 64 bit time.
>
> The mtx_timedlock is a wrapper on POSIX threads to provide C11 standard
> threads interface. It directly calls __pthread_mutex_timedlock64().
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  sysdeps/pthread/thrd_priv.h             |  4 +++
>  sysdeps/unix/sysv/linux/mtx_timedlock.c | 43 +++++++++++++++++++++++++
>  2 files changed, 47 insertions(+)
>  create mode 100644 sysdeps/unix/sysv/linux/mtx_timedlock.c
>
> diff --git a/sysdeps/pthread/thrd_priv.h b/sysdeps/pthread/thrd_priv.h
> index dbfec0df7a..efe453bdec 100644
> --- a/sysdeps/pthread/thrd_priv.h
> +++ b/sysdeps/pthread/thrd_priv.h
> @@ -26,10 +26,14 @@
>
>  #if __TIMESIZE == 64
>  # define __cnd_timedwait64 __cnd_timedwait
> +# define __mtx_timedlock64 __mtx_timedlock
>  #else
>  extern int __cnd_timedwait64 (cnd_t *restrict cond, mtx_t *restrict mutex,
>                                const struct __timespec64* restrict time_point);
>  libpthread_hidden_proto (__cnd_timedwait64)
> +extern int __mtx_timedlock64 (mtx_t *restrict mutex,
> +                              const struct __timespec64 *restrict time_point);
> +libpthread_hidden_proto (__mtx_timedlock64)
>  #endif
>
>  static __always_inline int
> diff --git a/sysdeps/unix/sysv/linux/mtx_timedlock.c b/sysdeps/unix/sysv/linux/mtx_timedlock.c
> new file mode 100644
> index 0000000000..ef57dd1e41
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/mtx_timedlock.c
> @@ -0,0 +1,43 @@
> +/* C11 threads mutex timed lock implementation - Linux variant.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <time.h>
> +#include "thrd_priv.h"
> +
> +int
> +__mtx_timedlock64 (mtx_t *restrict mutex,
> +                   const struct __timespec64 *restrict time_point)
> +{
> +  int err_code = __pthread_mutex_timedlock64 ((pthread_mutex_t *)mutex,
> +                                              time_point);
> +  return thrd_err_map (err_code);
> +}
> +
> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__mtx_timedlock64)
> +
> +int
> +__mtx_timedlock (mtx_t *restrict mutex,
> +                 const struct timespec *restrict time_point)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*time_point);
> +
> +  return __mtx_timedlock64(mutex, &ts64);
> +}
> +#endif
> +weak_alias (__mtx_timedlock, mtx_timedlock)
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] y2038: Convert thrd_sleep to support 64 bit time
  2020-11-03  9:14 ` [PATCH v2 3/3] y2038: Convert thrd_sleep " Lukasz Majewski
@ 2020-11-03 15:32   ` Alistair Francis
  2020-11-11 20:08   ` Adhemerval Zanella
  1 sibling, 0 replies; 20+ messages in thread
From: Alistair Francis @ 2020-11-03 15:32 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Joseph Myers, Paul Eggert, Adhemerval Zanella, Arnd Bergmann,
	Alistair Francis, GNU C Library, Florian Weimer,
	Carlos O'Donell, Stepan Golosunov, Andreas Schwab,
	Zack Weinberg

On Tue, Nov 3, 2020 at 1:15 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> The thrd_sleep function has been converted to support 64 bit time.
> It was also necessary to provide Linux specific copy of it to avoid
> problems on i686-gnu (i.e. HURD) port, which is not providing
> clock_nanosleep() supporting 64 bit time.
>
> The thrd_sleep is a wrapper on POSIX threads to provide C11 standard
> threads interface. It directly calls __clock_nanosleep64().
>
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  sysdeps/pthread/thrd_priv.h          |  4 ++
>  sysdeps/unix/sysv/linux/thrd_sleep.c | 55 ++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
>  create mode 100644 sysdeps/unix/sysv/linux/thrd_sleep.c
>
> diff --git a/sysdeps/pthread/thrd_priv.h b/sysdeps/pthread/thrd_priv.h
> index efe453bdec..c53d47a02c 100644
> --- a/sysdeps/pthread/thrd_priv.h
> +++ b/sysdeps/pthread/thrd_priv.h
> @@ -27,6 +27,7 @@
>  #if __TIMESIZE == 64
>  # define __cnd_timedwait64 __cnd_timedwait
>  # define __mtx_timedlock64 __mtx_timedlock
> +# define __thrd_sleep64 __thrd_sleep
>  #else
>  extern int __cnd_timedwait64 (cnd_t *restrict cond, mtx_t *restrict mutex,
>                                const struct __timespec64* restrict time_point);
> @@ -34,6 +35,9 @@ libpthread_hidden_proto (__cnd_timedwait64)
>  extern int __mtx_timedlock64 (mtx_t *restrict mutex,
>                                const struct __timespec64 *restrict time_point);
>  libpthread_hidden_proto (__mtx_timedlock64)
> +extern int __thrd_sleep64 (const struct __timespec64* time_point,
> +                           struct __timespec64* remaining);
> +libpthread_hidden_proto (__thrd_sleep64)
>  #endif
>
>  static __always_inline int
> diff --git a/sysdeps/unix/sysv/linux/thrd_sleep.c b/sysdeps/unix/sysv/linux/thrd_sleep.c
> new file mode 100644
> index 0000000000..632527d26a
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/thrd_sleep.c
> @@ -0,0 +1,55 @@
> +/* C11 threads thread sleep implementation - Linux variant.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <time.h>
> +#include <sysdep-cancel.h>
> +
> +#include "thrd_priv.h"
> +
> +int
> +__thrd_sleep64 (const struct __timespec64* time_point,
> +                struct __timespec64* remaining)
> +{
> +  int ret = __clock_nanosleep_time64 (CLOCK_REALTIME, 0, time_point, remaining);
> +  /* C11 states thrd_sleep function returns -1 if it has been interrupted
> +     by a signal, or a negative value if it fails.  */
> +  switch (ret)
> +  {
> +     case 0:      return 0;
> +     case EINTR:  return -1;
> +     default:     return -2;
> +  }
> +}
> +
> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__thrd_sleep64)
> +
> +int
> +__thrd_sleep (const struct timespec* time_point, struct timespec* remaining)
> +{
> +  const struct __timespec64 treq64 = valid_timespec_to_timespec64 (*time_point);
> +  struct __timespec64 trem64;
> +
> +  int ret = __thrd_sleep64 (&treq64, &trem64);
> +  if (ret == -1 && remaining != NULL)
> +    *remaining = valid_timespec64_to_timespec (trem64);
> +
> +  return ret;
> +}
> +#endif
> +weak_alias (__thrd_sleep, thrd_sleep)
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX
  2020-11-03 14:45   ` [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX Chen Li
@ 2020-11-03 16:43     ` Adhemerval Zanella
  2020-11-03 16:49       ` Florian Weimer
  0 siblings, 1 reply; 20+ messages in thread
From: Adhemerval Zanella @ 2020-11-03 16:43 UTC (permalink / raw)
  To: Chen Li, GNU C Library, Florian Weimer



On 03/11/2020 11:45, Chen Li wrote:
> 
> On  Mon Oct 26 19:44:44 GMT 2020,
> Adhemerval Zanella  wrote:
> 
>> I think it would be good to refactor the code to use a PATH_MAX variable
>> instead, something like:
>>
>>  _Bool
>> shm_get_name (const char *prefix, char *shm_name, size shm_path_max)
> 
> So you prefer function over macro here? If so, shm_dir, shm_dirlen, shm_name
> should all be passed as args too, because unlike c macro, function is not
> dynamical scope. Given such long arg list(errno_for_invalid, retval_for_invalid,
> prefix, shm_dir, shm_dirlen, shm_name), I'm not sure weathre it confrom glibc's
> coding style, and is refactoring to split into more small functions a good idea? 

Florian has pointed me out a better solution would be use
use his fix [1] for BZ#25383 [2] which refactor the POSIX sem/shm
code to use a static buffer instead.

Then the ENAMETOOLONG could be returned by '__shm_get_name'
instead.

Florian, do you plan to send your patch for review?

[1] https://sourceware.org/bugzilla/attachment.cgi?id=12921
[2] https://sourceware.org/bugzilla/show_bug.cgi?id=25383

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX
  2020-11-03 16:43     ` Adhemerval Zanella
@ 2020-11-03 16:49       ` Florian Weimer
  2020-11-03 16:57         ` Adhemerval Zanella
  0 siblings, 1 reply; 20+ messages in thread
From: Florian Weimer @ 2020-11-03 16:49 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: Chen Li, GNU C Library

* Adhemerval Zanella:

> Florian has pointed me out a better solution would be use
> use his fix [1] for BZ#25383 [2] which refactor the POSIX sem/shm
> code to use a static buffer instead.
>
> Then the ENAMETOOLONG could be returned by '__shm_get_name'
> instead.
>
> Florian, do you plan to send your patch for review?

I'm working on the change you requested (eliminate the new struct
type).  No ETA yet.  Hopefully still this month.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX
  2020-11-03 16:49       ` Florian Weimer
@ 2020-11-03 16:57         ` Adhemerval Zanella
  0 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2020-11-03 16:57 UTC (permalink / raw)
  To: Florian Weimer; +Cc: Chen Li, GNU C Library



On 03/11/2020 13:49, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> Florian has pointed me out a better solution would be use
>> use his fix [1] for BZ#25383 [2] which refactor the POSIX sem/shm
>> code to use a static buffer instead.
>>
>> Then the ENAMETOOLONG could be returned by '__shm_get_name'
>> instead.
>>
>> Florian, do you plan to send your patch for review?
> 
> I'm working on the change you requested (eliminate the new struct
> type).  No ETA yet.  Hopefully still this month.

Thanks, I think the NAME_MAX errno fix should be simpler on top of
your patch.


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/3] y2038: Convert cnd_timedwait to support 64 bit time
  2020-11-03  9:14 [PATCH v2 1/3] y2038: Convert cnd_timedwait to support 64 bit time Lukasz Majewski
                   ` (2 preceding siblings ...)
  2020-11-03 15:24 ` [PATCH v2 1/3] y2038: Convert cnd_timedwait " Alistair Francis
@ 2020-11-11 20:05 ` Adhemerval Zanella
  3 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2020-11-11 20:05 UTC (permalink / raw)
  To: Lukasz Majewski, Joseph Myers, Paul Eggert
  Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
	Florian Weimer, Carlos O'Donell, Stepan Golosunov,
	Andreas Schwab, Zack Weinberg



On 03/11/2020 06:14, Lukasz Majewski wrote:
> The cnd_timedwait function has been converted to support 64 bit time.
> It was also necessary to provide Linux specific copy of it to avoid
> problems on i686-gnu (i.e. HURD) port, which is not providing
> pthread_cond_timedwait() supporting 64 bit time.
> 
> The cnd_timedwait is a wrapper on POSIX threads to provide C11 standard
> threads interface. It directly calls __pthread_cond_timedwait64().
> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
> ---
>  sysdeps/pthread/thrd_priv.h             |  8 +++++
>  sysdeps/unix/sysv/linux/cnd_timedwait.c | 44 +++++++++++++++++++++++++
>  2 files changed, 52 insertions(+)
>  create mode 100644 sysdeps/unix/sysv/linux/cnd_timedwait.c
> 
> diff --git a/sysdeps/pthread/thrd_priv.h b/sysdeps/pthread/thrd_priv.h
> index d22ad6f632..dbfec0df7a 100644
> --- a/sysdeps/pthread/thrd_priv.h
> +++ b/sysdeps/pthread/thrd_priv.h
> @@ -24,6 +24,14 @@
>  #include <errno.h>
>  #include "pthreadP.h"	/* For pthread_{mutex,cond}_t definitions.  */
>  
> +#if __TIMESIZE == 64
> +# define __cnd_timedwait64 __cnd_timedwait
> +#else
> +extern int __cnd_timedwait64 (cnd_t *restrict cond, mtx_t *restrict mutex,
> +                              const struct __timespec64* restrict time_point);
> +libpthread_hidden_proto (__cnd_timedwait64)
> +#endif
> +
>  static __always_inline int
>  thrd_err_map (int err_code)
>  {

Although not really incorrect, since you we pushing a Linux only cnd_timedwait;
I think __cnd_timedwait64 should be made Linux only as well (to avoid Hurd
having a dangling __cnd_timewait64 definition).  The usual way is to add
a threads.h internal headers similar to the one for Linux sys/msg.h
(sysdeps/unix/sysv/linux/include/sys/msg.h).

> diff --git a/sysdeps/unix/sysv/linux/cnd_timedwait.c b/sysdeps/unix/sysv/linux/cnd_timedwait.c
> new file mode 100644
> index 0000000000..5bf5a2d968
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/cnd_timedwait.c
> @@ -0,0 +1,44 @@
> +/* C11 threads conditional timed wait implementation - Linux variant.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <time.h>
> +#include "thrd_priv.h"
> +
> +int
> +__cnd_timedwait64 (cnd_t *restrict cond, mtx_t *restrict mutex,
> +                   const struct __timespec64* restrict time_point)
> +{
> +  int err_code = __pthread_cond_timedwait64 ((pthread_cond_t *) cond,
> +                                             (pthread_mutex_t *) mutex,
> +                                             time_point);
> +  return thrd_err_map (err_code);
> +}
> +
> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__cnd_timedwait64)
> +
> +int
> +__cnd_timedwait (cnd_t *restrict cond, mtx_t *restrict mutex,
> +                 const struct timespec* restrict time_point)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*time_point);
> +
> +  return __cnd_timedwait64(cond, mutex, &ts64);
> +}
> +#endif
> +weak_alias (__cnd_timedwait, cnd_timedwait)
> 

Ok.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 2/3] y2038: Convert mtx_timedlock to support 64 bit time
  2020-11-03  9:14 ` [PATCH v2 2/3] y2038: Convert mtx_timedlock " Lukasz Majewski
  2020-11-03 14:45   ` [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX Chen Li
  2020-11-03 15:28   ` [PATCH v2 2/3] y2038: Convert mtx_timedlock to support 64 bit time Alistair Francis
@ 2020-11-11 20:06   ` Adhemerval Zanella
  2 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2020-11-11 20:06 UTC (permalink / raw)
  To: Lukasz Majewski, Joseph Myers, Paul Eggert
  Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
	Florian Weimer, Carlos O'Donell, Stepan Golosunov,
	Andreas Schwab, Zack Weinberg



On 03/11/2020 06:14, Lukasz Majewski wrote:
> The mtx_timedlock function has been converted to support 64 bit time.
> It was also necessary to provide Linux specific copy of it to avoid
> problems on i686-gnu (i.e. HURD) port, which is not providing
> pthread_mutex_timedlock() supporting 64 bit time.
> 
> The mtx_timedlock is a wrapper on POSIX threads to provide C11 standard
> threads interface. It directly calls __pthread_mutex_timedlock64().
> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
> ---
>  sysdeps/pthread/thrd_priv.h             |  4 +++
>  sysdeps/unix/sysv/linux/mtx_timedlock.c | 43 +++++++++++++++++++++++++
>  2 files changed, 47 insertions(+)
>  create mode 100644 sysdeps/unix/sysv/linux/mtx_timedlock.c
> 
> diff --git a/sysdeps/pthread/thrd_priv.h b/sysdeps/pthread/thrd_priv.h
> index dbfec0df7a..efe453bdec 100644
> --- a/sysdeps/pthread/thrd_priv.h
> +++ b/sysdeps/pthread/thrd_priv.h
> @@ -26,10 +26,14 @@
>  
>  #if __TIMESIZE == 64
>  # define __cnd_timedwait64 __cnd_timedwait
> +# define __mtx_timedlock64 __mtx_timedlock
>  #else
>  extern int __cnd_timedwait64 (cnd_t *restrict cond, mtx_t *restrict mutex,
>                                const struct __timespec64* restrict time_point);
>  libpthread_hidden_proto (__cnd_timedwait64)
> +extern int __mtx_timedlock64 (mtx_t *restrict mutex,
> +                              const struct __timespec64 *restrict time_point);
> +libpthread_hidden_proto (__mtx_timedlock64)
>  #endif
>  
>  static __always_inline int


Same as previous patch, I think this should be moved to a Linux internal
thread.h header.

> diff --git a/sysdeps/unix/sysv/linux/mtx_timedlock.c b/sysdeps/unix/sysv/linux/mtx_timedlock.c
> new file mode 100644
> index 0000000000..ef57dd1e41
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/mtx_timedlock.c
> @@ -0,0 +1,43 @@
> +/* C11 threads mutex timed lock implementation - Linux variant.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <time.h>
> +#include "thrd_priv.h"
> +
> +int
> +__mtx_timedlock64 (mtx_t *restrict mutex,
> +                   const struct __timespec64 *restrict time_point)
> +{
> +  int err_code = __pthread_mutex_timedlock64 ((pthread_mutex_t *)mutex,
> +                                              time_point);
> +  return thrd_err_map (err_code);
> +}
> +
> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__mtx_timedlock64)
> +
> +int
> +__mtx_timedlock (mtx_t *restrict mutex,
> +                 const struct timespec *restrict time_point)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*time_point);
> +
> +  return __mtx_timedlock64(mutex, &ts64);

Space after function name.

> +}
> +#endif
> +weak_alias (__mtx_timedlock, mtx_timedlock)
> 

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] y2038: Convert thrd_sleep to support 64 bit time
  2020-11-03  9:14 ` [PATCH v2 3/3] y2038: Convert thrd_sleep " Lukasz Majewski
  2020-11-03 15:32   ` Alistair Francis
@ 2020-11-11 20:08   ` Adhemerval Zanella
  1 sibling, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2020-11-11 20:08 UTC (permalink / raw)
  To: Lukasz Majewski, Joseph Myers, Paul Eggert
  Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
	Florian Weimer, Carlos O'Donell, Stepan Golosunov,
	Andreas Schwab, Zack Weinberg



On 03/11/2020 06:14, Lukasz Majewski wrote:
> The thrd_sleep function has been converted to support 64 bit time.
> It was also necessary to provide Linux specific copy of it to avoid
> problems on i686-gnu (i.e. HURD) port, which is not providing
> clock_nanosleep() supporting 64 bit time.
> 
> The thrd_sleep is a wrapper on POSIX threads to provide C11 standard
> threads interface. It directly calls __clock_nanosleep64().
> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
> ---
>  sysdeps/pthread/thrd_priv.h          |  4 ++
>  sysdeps/unix/sysv/linux/thrd_sleep.c | 55 ++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
>  create mode 100644 sysdeps/unix/sysv/linux/thrd_sleep.c
> 
> diff --git a/sysdeps/pthread/thrd_priv.h b/sysdeps/pthread/thrd_priv.h
> index efe453bdec..c53d47a02c 100644
> --- a/sysdeps/pthread/thrd_priv.h
> +++ b/sysdeps/pthread/thrd_priv.h
> @@ -27,6 +27,7 @@
>  #if __TIMESIZE == 64
>  # define __cnd_timedwait64 __cnd_timedwait
>  # define __mtx_timedlock64 __mtx_timedlock
> +# define __thrd_sleep64 __thrd_sleep
>  #else
>  extern int __cnd_timedwait64 (cnd_t *restrict cond, mtx_t *restrict mutex,
>                                const struct __timespec64* restrict time_point);
> @@ -34,6 +35,9 @@ libpthread_hidden_proto (__cnd_timedwait64)
>  extern int __mtx_timedlock64 (mtx_t *restrict mutex,
>                                const struct __timespec64 *restrict time_point);
>  libpthread_hidden_proto (__mtx_timedlock64)
> +extern int __thrd_sleep64 (const struct __timespec64* time_point,
> +                           struct __timespec64* remaining);
> +libpthread_hidden_proto (__thrd_sleep64)
>  #endif
>  
>  static __always_inline int

As previous patches, I think it should be moved to a Linux internal thread.h
header.

> diff --git a/sysdeps/unix/sysv/linux/thrd_sleep.c b/sysdeps/unix/sysv/linux/thrd_sleep.c
> new file mode 100644
> index 0000000000..632527d26a
> --- /dev/null
> +++ b/sysdeps/unix/sysv/linux/thrd_sleep.c
> @@ -0,0 +1,55 @@
> +/* C11 threads thread sleep implementation - Linux variant.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <time.h>
> +#include <sysdep-cancel.h>
> +
> +#include "thrd_priv.h"
> +
> +int
> +__thrd_sleep64 (const struct __timespec64* time_point,
> +                struct __timespec64* remaining)
> +{
> +  int ret = __clock_nanosleep_time64 (CLOCK_REALTIME, 0, time_point, remaining);

Line too long.

> +  /* C11 states thrd_sleep function returns -1 if it has been interrupted
> +     by a signal, or a negative value if it fails.  */
> +  switch (ret)
> +  {
> +     case 0:      return 0;
> +     case EINTR:  return -1;
> +     default:     return -2;
> +  }
> +}
> +
> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__thrd_sleep64)
> +
> +int
> +__thrd_sleep (const struct timespec* time_point, struct timespec* remaining)
> +{
> +  const struct __timespec64 treq64 = valid_timespec_to_timespec64 (*time_point);
> +  struct __timespec64 trem64;
> +
> +  int ret = __thrd_sleep64 (&treq64, &trem64);
> +  if (ret == -1 && remaining != NULL)
> +    *remaining = valid_timespec64_to_timespec (trem64);
> +
> +  return ret;
> +}
> +#endif
> +weak_alias (__thrd_sleep, thrd_sleep)
> 

Ok.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] y2038: Convert thrd_sleep to support 64 bit time
  2020-11-12  9:59 ` [PATCH v2 3/3] y2038: Convert thrd_sleep " Lukasz Majewski
  2020-11-12 10:22   ` Andreas Schwab
@ 2020-11-12 10:26   ` Andreas Schwab
  1 sibling, 0 replies; 20+ messages in thread
From: Andreas Schwab @ 2020-11-12 10:26 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Joseph Myers, Paul Eggert, Adhemerval Zanella, Florian Weimer,
	GNU C Library, Stepan Golosunov, Alistair Francis

On Nov 12 2020, Lukasz Majewski wrote:

> +int
> +__thrd_sleep (const struct timespec* time_point, struct timespec* remaining)
> +{
> +  const struct __timespec64 treq64 = valid_timespec_to_timespec64 (*time_point);
> +  struct __timespec64 trem64;
> +
> +  int ret = __thrd_sleep64 (&treq64, &trem64);
> +  if (ret == -1 && remaining != NULL)
> +    *remaining = valid_timespec64_to_timespec (trem64);

If remaining is NULL then __thrd_sleep64 should probably be called with
NULL.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 3/3] y2038: Convert thrd_sleep to support 64 bit time
  2020-11-12  9:59 ` [PATCH v2 3/3] y2038: Convert thrd_sleep " Lukasz Majewski
@ 2020-11-12 10:22   ` Andreas Schwab
  2020-11-12 10:26   ` Andreas Schwab
  1 sibling, 0 replies; 20+ messages in thread
From: Andreas Schwab @ 2020-11-12 10:22 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Joseph Myers, Paul Eggert, Adhemerval Zanella, Florian Weimer,
	GNU C Library, Stepan Golosunov, Alistair Francis

On Nov 12 2020, Lukasz Majewski wrote:

> @@ -28,4 +29,7 @@ libpthread_hidden_proto (__cnd_timedwait64)
>  extern int __mtx_timedlock64 (mtx_t *restrict mutex,
>                                const struct __timespec64 *restrict time_point);
>  libpthread_hidden_proto (__mtx_timedlock64)
> +extern int __thrd_sleep64 (const struct __timespec64* time_point,
> +                           struct __timespec64* remaining);

Style: space before asterisk, not after.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 3/3] y2038: Convert thrd_sleep to support 64 bit time
  2020-11-12  9:59 Lukasz Majewski
@ 2020-11-12  9:59 ` Lukasz Majewski
  2020-11-12 10:22   ` Andreas Schwab
  2020-11-12 10:26   ` Andreas Schwab
  0 siblings, 2 replies; 20+ messages in thread
From: Lukasz Majewski @ 2020-11-12  9:59 UTC (permalink / raw)
  To: Joseph Myers, Paul Eggert, Adhemerval Zanella
  Cc: Alistair Francis, Arnd Bergmann, Alistair Francis, GNU C Library,
	Florian Weimer, Carlos O'Donell, Stepan Golosunov,
	Andreas Schwab, Zack Weinberg, Lukasz Majewski

The thrd_sleep function has been converted to support 64 bit time.
It was also necessary to provide Linux specific copy of it to avoid
problems on i686-gnu (i.e. HURD) port, which is not providing
clock_nanosleep() supporting 64 bit time.

The thrd_sleep is a wrapper on POSIX threads to provide C11 standard
threads interface. It directly calls __clock_nanosleep64().

Build tests:
./src/scripts/build-many-glibcs.py glibcs
---
Changes for v2:
- Add aliasing and __thrd_sleep64 definition to linux specific
  version of thrd_priv.h
- Reformat the thrd_sleep.c to not exceed the line width

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 sysdeps/unix/sysv/linux/thrd_priv.h  |  4 ++
 sysdeps/unix/sysv/linux/thrd_sleep.c | 56 ++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)
 create mode 100644 sysdeps/unix/sysv/linux/thrd_sleep.c

diff --git a/sysdeps/unix/sysv/linux/thrd_priv.h b/sysdeps/unix/sysv/linux/thrd_priv.h
index d7851f0986..3cfb529c2a 100644
--- a/sysdeps/unix/sysv/linux/thrd_priv.h
+++ b/sysdeps/unix/sysv/linux/thrd_priv.h
@@ -21,6 +21,7 @@
 #if __TIMESIZE == 64
 # define __cnd_timedwait64 __cnd_timedwait
 # define __mtx_timedlock64 __mtx_timedlock
+# define __thrd_sleep64 __thrd_sleep
 #else
 extern int __cnd_timedwait64 (cnd_t *restrict cond, mtx_t *restrict mutex,
                               const struct __timespec64* restrict time_point);
@@ -28,4 +29,7 @@ libpthread_hidden_proto (__cnd_timedwait64)
 extern int __mtx_timedlock64 (mtx_t *restrict mutex,
                               const struct __timespec64 *restrict time_point);
 libpthread_hidden_proto (__mtx_timedlock64)
+extern int __thrd_sleep64 (const struct __timespec64* time_point,
+                           struct __timespec64* remaining);
+libpthread_hidden_proto (__thrd_sleep64)
 #endif
diff --git a/sysdeps/unix/sysv/linux/thrd_sleep.c b/sysdeps/unix/sysv/linux/thrd_sleep.c
new file mode 100644
index 0000000000..32dbf2b772
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/thrd_sleep.c
@@ -0,0 +1,56 @@
+/* C11 threads thread sleep implementation - Linux variant.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <time.h>
+#include <sysdep-cancel.h>
+
+#include "thrd_priv.h"
+
+int
+__thrd_sleep64 (const struct __timespec64* time_point,
+                struct __timespec64* remaining)
+{
+  int ret = __clock_nanosleep_time64 (CLOCK_REALTIME, 0, time_point,
+                                      remaining);
+  /* C11 states thrd_sleep function returns -1 if it has been interrupted
+     by a signal, or a negative value if it fails.  */
+  switch (ret)
+  {
+     case 0:      return 0;
+     case EINTR:  return -1;
+     default:     return -2;
+  }
+}
+
+#if __TIMESIZE != 64
+libpthread_hidden_def (__thrd_sleep64)
+
+int
+__thrd_sleep (const struct timespec* time_point, struct timespec* remaining)
+{
+  const struct __timespec64 treq64 = valid_timespec_to_timespec64 (*time_point);
+  struct __timespec64 trem64;
+
+  int ret = __thrd_sleep64 (&treq64, &trem64);
+  if (ret == -1 && remaining != NULL)
+    *remaining = valid_timespec64_to_timespec (trem64);
+
+  return ret;
+}
+#endif
+weak_alias (__thrd_sleep, thrd_sleep)
-- 
2.20.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2020-11-12 10:26 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-03  9:14 [PATCH v2 1/3] y2038: Convert cnd_timedwait to support 64 bit time Lukasz Majewski
2020-11-03  9:14 ` [PATCH v2 2/3] y2038: Convert mtx_timedlock " Lukasz Majewski
2020-11-03 14:45   ` [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX Chen Li
2020-11-03 16:43     ` Adhemerval Zanella
2020-11-03 16:49       ` Florian Weimer
2020-11-03 16:57         ` Adhemerval Zanella
2020-11-03 15:28   ` [PATCH v2 2/3] y2038: Convert mtx_timedlock to support 64 bit time Alistair Francis
2020-11-11 20:06   ` Adhemerval Zanella
2020-11-03  9:14 ` [PATCH v2 3/3] y2038: Convert thrd_sleep " Lukasz Majewski
2020-11-03 15:32   ` Alistair Francis
2020-11-11 20:08   ` Adhemerval Zanella
2020-11-03 15:24 ` [PATCH v2 1/3] y2038: Convert cnd_timedwait " Alistair Francis
2020-11-11 20:05 ` Adhemerval Zanella
  -- strict thread matches above, loose matches on Subject: below --
2020-11-12  9:59 Lukasz Majewski
2020-11-12  9:59 ` [PATCH v2 3/3] y2038: Convert thrd_sleep " Lukasz Majewski
2020-11-12 10:22   ` Andreas Schwab
2020-11-12 10:26   ` Andreas Schwab
2020-10-16 10:09 [PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX Chen Li
2020-10-26 19:44 ` Adhemerval Zanella
2020-10-27 10:27   ` Florian Weimer
2020-10-27 11:53     ` Adhemerval Zanella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).