This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 02/14] Consolidate non cancellable read call
Since this is a refactor, if no one opposes I will commit it shortly.
On 03/08/2017 10:11, Adhemerval Zanella wrote:
> This patch consolidates all the non cancellable read calls to use
> the __read_nocancel identifier. For non cancellable targets it will
> be just a macro to call the default respective symbol while on Linux
> will be a internal one.
>
> Also, since it is used on libcrypto it is exported in GLIBC_PRIVATE
> namespace.
>
> Checked on x86_64-linux-gnu, x86_64-linux-gnu-x32, and i686-linux-gnu.
>
> * sysdeps/generic/not-cancel.h (read_not_cancel): Remove macro.
> (__read_nocancel): New macro.
> * sysdeps/unix/sysv/linux/Versions (libc) [GLIBC_PRIVATE]: Add
> __read_nocancel.
> * sysdeps/unix/sysv/linux/not-cancel.h (__read_nocancel): Remove
> macro.
> (__read_nocancel): New prototype.
> * sysdeps/unix/sysv/linux/read.c (__read_nocancel): New function.
> * catgets/open_catalog.c (__open_catalog): Replace read_not_cancel
> with __read_nocancel.
> * intl/loadmsgcat.c (read): Likewise.
> * libio/fileops.c (_IO_file_read): Likewise.
> * locale/loadlocale.c (_nl_load_locale): Likewise.
> * login/utmp_file.c (getutent_r_file): Likewise.
> (internal_getut_r): Likewise.
> (getutline_r_file): Likewise.
> * sysdeps/unix/sysv/linux/fips-private.h (fips_enable_p): Likewise.
> * sysdeps/unix/sysv/linux/gethostid.c (gethostid): Likewise.
> * sysdeps/unix/sysv/linux/getloadavg.c (getloadavg): Likewise.
> * sysdeps/unix/sysv/linux/getlogin_r.c (__getlogin_r_loginuid):
> Likewise.
> * sysdeps/unix/sysv/linux/getsysstats.c (next_line): Likewise.
> * sysdeps/unix/sysv/linux/i386/smp.h (is_smp_system): Likewise.
> * sysdeps/unix/sysv/linux/ia64/has_cpuclock.c (has_cpuclock):
> Likewise.
> * sysdeps/unix/sysv/linux/libc_fatal.c (backtrace_and_maps):
> Likewise.
> * sysdeps/unix/sysv/linux/malloc-sysdep.h (check_may_shrink_heap):
> Likewise.
> * sysdeps/unix/sysv/linux/pthread_getname.c (pthread_getname_np):
> Likewise.
> * sysdeps/unix/sysv/linux/sysconf.c (__sysconf): Likewise.
> ---
> ChangeLog | 33 +++++++++++++++++++++++++++++
> catgets/open_catalog.c | 2 +-
> intl/loadmsgcat.c | 2 +-
> libio/fileops.c | 2 +-
> locale/loadlocale.c | 2 +-
> login/utmp_file.c | 8 +++----
> sysdeps/generic/not-cancel.h | 2 +-
> sysdeps/unix/sysv/linux/Versions | 1 +
> sysdeps/unix/sysv/linux/fips-private.h | 2 +-
> sysdeps/unix/sysv/linux/gethostid.c | 2 +-
> sysdeps/unix/sysv/linux/getloadavg.c | 2 +-
> sysdeps/unix/sysv/linux/getlogin_r.c | 2 +-
> sysdeps/unix/sysv/linux/getsysstats.c | 4 ++--
> sysdeps/unix/sysv/linux/i386/smp.h | 2 +-
> sysdeps/unix/sysv/linux/ia64/has_cpuclock.c | 2 +-
> sysdeps/unix/sysv/linux/libc_fatal.c | 2 +-
> sysdeps/unix/sysv/linux/malloc-sysdep.h | 2 +-
> sysdeps/unix/sysv/linux/not-cancel.h | 10 +++------
> sysdeps/unix/sysv/linux/pthread_getname.c | 2 +-
> sysdeps/unix/sysv/linux/read.c | 12 +++++++++++
> sysdeps/unix/sysv/linux/sysconf.c | 2 +-
> 21 files changed, 70 insertions(+), 28 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index 04e9f71..5199a69 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,38 @@
> 2017-08-02 Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> + * sysdeps/generic/not-cancel.h (read_not_cancel): Remove macro.
> + (__read_nocancel): New macro.
> + * sysdeps/unix/sysv/linux/Versions (libc) [GLIBC_PRIVATE]: Add
> + __read_nocancel.
> + * sysdeps/unix/sysv/linux/not-cancel.h (__read_nocancel): Remove
> + macro.
> + (__read_nocancel): New prototype.
> + * sysdeps/unix/sysv/linux/read.c (__read_nocancel): New function.
> + * catgets/open_catalog.c (__open_catalog): Replace read_not_cancel
> + with __read_nocancel.
> + * intl/loadmsgcat.c (read): Likewise.
> + * libio/fileops.c (_IO_file_read): Likewise.
> + * locale/loadlocale.c (_nl_load_locale): Likewise.
> + * login/utmp_file.c (getutent_r_file): Likewise.
> + (internal_getut_r): Likewise.
> + (getutline_r_file): Likewise.
> + * sysdeps/unix/sysv/linux/fips-private.h (fips_enable_p): Likewise.
> + * sysdeps/unix/sysv/linux/gethostid.c (gethostid): Likewise.
> + * sysdeps/unix/sysv/linux/getloadavg.c (getloadavg): Likewise.
> + * sysdeps/unix/sysv/linux/getlogin_r.c (__getlogin_r_loginuid):
> + Likewise.
> + * sysdeps/unix/sysv/linux/getsysstats.c (next_line): Likewise.
> + * sysdeps/unix/sysv/linux/i386/smp.h (is_smp_system): Likewise.
> + * sysdeps/unix/sysv/linux/ia64/has_cpuclock.c (has_cpuclock):
> + Likewise.
> + * sysdeps/unix/sysv/linux/libc_fatal.c (backtrace_and_maps):
> + Likewise.
> + * sysdeps/unix/sysv/linux/malloc-sysdep.h (check_may_shrink_heap):
> + Likewise.
> + * sysdeps/unix/sysv/linux/pthread_getname.c (pthread_getname_np):
> + Likewise.
> + * sysdeps/unix/sysv/linux/sysconf.c (__sysconf): Likewise.
> +
> * sysdeps/generic/not-cancel.h (open_not_cancel): Remove macro.
> (open_not_cancel_2): Likewise.
> (open_nocancel): New macro.
> diff --git a/catgets/open_catalog.c b/catgets/open_catalog.c
> index 4621b26..d79a6b1 100644
> --- a/catgets/open_catalog.c
> +++ b/catgets/open_catalog.c
> @@ -237,7 +237,7 @@ __open_catalog (const char *cat_name, const char *nlspath, const char *env_var,
> /* Save read, handle partial reads. */
> do
> {
> - size_t now = read_not_cancel (fd, (((char *) catalog->file_ptr)
> + size_t now = __read_nocancel (fd, (((char *) catalog->file_ptr)
> + (st.st_size - todo)), todo);
> if (now == 0 || now == (size_t) -1)
> {
> diff --git a/intl/loadmsgcat.c b/intl/loadmsgcat.c
> index adca419..514e38f 100644
> --- a/intl/loadmsgcat.c
> +++ b/intl/loadmsgcat.c
> @@ -447,7 +447,7 @@
> file and the name space must not be polluted. */
> # define open(name, flags) __open_nocancel (name, flags)
> # define close(fd) close_not_cancel_no_status (fd)
> -# define read(fd, buf, n) read_not_cancel (fd, buf, n)
> +# define read(fd, buf, n) __read_nocancel (fd, buf, n)
> # define mmap(addr, len, prot, flags, fd, offset) \
> __mmap (addr, len, prot, flags, fd, offset)
> # define munmap(addr, len) __munmap (addr, len)
> diff --git a/libio/fileops.c b/libio/fileops.c
> index 67f3d72..80bd3f5 100644
> --- a/libio/fileops.c
> +++ b/libio/fileops.c
> @@ -1204,7 +1204,7 @@ _IO_ssize_t
> _IO_file_read (_IO_FILE *fp, void *buf, _IO_ssize_t size)
> {
> return (__builtin_expect (fp->_flags2 & _IO_FLAGS2_NOTCANCEL, 0)
> - ? read_not_cancel (fp->_fileno, buf, size)
> + ? __read_nocancel (fp->_fileno, buf, size)
> : read (fp->_fileno, buf, size));
> }
> libc_hidden_def (_IO_file_read)
> diff --git a/locale/loadlocale.c b/locale/loadlocale.c
> index 9c69ad4..a733557 100644
> --- a/locale/loadlocale.c
> +++ b/locale/loadlocale.c
> @@ -238,7 +238,7 @@ _nl_load_locale (struct loaded_l10nfile *file, int category)
> char *p = (char *) filedata;
> while (to_read > 0)
> {
> - nread = read_not_cancel (fd, p, to_read);
> + nread = __read_nocancel (fd, p, to_read);
> if (__builtin_expect (nread, 1) <= 0)
> {
> free (filedata);
> diff --git a/login/utmp_file.c b/login/utmp_file.c
> index 1407116..61d03d6 100644
> --- a/login/utmp_file.c
> +++ b/login/utmp_file.c
> @@ -187,7 +187,7 @@ getutent_r_file (struct utmp *buffer, struct utmp **result)
> }
>
> /* Read the next entry. */
> - nbytes = read_not_cancel (file_fd, &last_entry, sizeof (struct utmp));
> + nbytes = __read_nocancel (file_fd, &last_entry, sizeof (struct utmp));
>
> UNLOCK_FILE (file_fd);
>
> @@ -231,7 +231,7 @@ internal_getut_r (const struct utmp *id, struct utmp *buffer,
> while (1)
> {
> /* Read the next entry. */
> - if (read_not_cancel (file_fd, buffer, sizeof (struct utmp))
> + if (__read_nocancel (file_fd, buffer, sizeof (struct utmp))
> != sizeof (struct utmp))
> {
> __set_errno (ESRCH);
> @@ -253,7 +253,7 @@ internal_getut_r (const struct utmp *id, struct utmp *buffer,
> while (1)
> {
> /* Read the next entry. */
> - if (read_not_cancel (file_fd, buffer, sizeof (struct utmp))
> + if (__read_nocancel (file_fd, buffer, sizeof (struct utmp))
> != sizeof (struct utmp))
> {
> __set_errno (ESRCH);
> @@ -329,7 +329,7 @@ getutline_r_file (const struct utmp *line, struct utmp *buffer,
> while (1)
> {
> /* Read the next entry. */
> - if (read_not_cancel (file_fd, &last_entry, sizeof (struct utmp))
> + if (__read_nocancel (file_fd, &last_entry, sizeof (struct utmp))
> != sizeof (struct utmp))
> {
> __set_errno (ESRCH);
> diff --git a/sysdeps/generic/not-cancel.h b/sysdeps/generic/not-cancel.h
> index a4f7b7b..4fada2f 100644
> --- a/sysdeps/generic/not-cancel.h
> +++ b/sysdeps/generic/not-cancel.h
> @@ -34,7 +34,7 @@
> __close (fd)
> #define close_not_cancel_no_status(fd) \
> (void) __close (fd)
> -#define read_not_cancel(fd, buf, n) \
> +#define __read_nocancel(fd, buf, n) \
> __read (fd, buf, n)
> #define write_not_cancel(fd, buf, n) \
> __write (fd, buf, n)
> diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions
> index 3c64077..b553514 100644
> --- a/sysdeps/unix/sysv/linux/Versions
> +++ b/sysdeps/unix/sysv/linux/Versions
> @@ -170,6 +170,7 @@ libc {
> # functions used in other libraries
> __syscall_rt_sigqueueinfo;
> __open_nocancel;
> + __read_nocancel;
> # functions used by nscd
> __netlink_assert_response;
> }
> diff --git a/sysdeps/unix/sysv/linux/fips-private.h b/sysdeps/unix/sysv/linux/fips-private.h
> index 3a83a0f..775f2c2 100644
> --- a/sysdeps/unix/sysv/linux/fips-private.h
> +++ b/sysdeps/unix/sysv/linux/fips-private.h
> @@ -49,7 +49,7 @@ fips_enabled_p (void)
> /* This is more than enough, the file contains a single integer. */
> char buf[32];
> ssize_t n;
> - n = TEMP_FAILURE_RETRY (read_not_cancel (fd, buf, sizeof (buf) - 1));
> + n = TEMP_FAILURE_RETRY (__read_nocancel (fd, buf, sizeof (buf) - 1));
> close_not_cancel_no_status (fd);
>
> if (n > 0)
> diff --git a/sysdeps/unix/sysv/linux/gethostid.c b/sysdeps/unix/sysv/linux/gethostid.c
> index 3ea8656..6e19dee 100644
> --- a/sysdeps/unix/sysv/linux/gethostid.c
> +++ b/sysdeps/unix/sysv/linux/gethostid.c
> @@ -80,7 +80,7 @@ gethostid (void)
> fd = __open_nocancel (HOSTIDFILE, O_RDONLY|O_LARGEFILE, 0);
> if (fd >= 0)
> {
> - ssize_t n = read_not_cancel (fd, &id, sizeof (id));
> + ssize_t n = __read_nocancel (fd, &id, sizeof (id));
>
> close_not_cancel_no_status (fd);
>
> diff --git a/sysdeps/unix/sysv/linux/getloadavg.c b/sysdeps/unix/sysv/linux/getloadavg.c
> index 0bfe41d..64bad17 100644
> --- a/sysdeps/unix/sysv/linux/getloadavg.c
> +++ b/sysdeps/unix/sysv/linux/getloadavg.c
> @@ -42,7 +42,7 @@ getloadavg (double loadavg[], int nelem)
> ssize_t nread;
> int i;
>
> - nread = read_not_cancel (fd, buf, sizeof buf - 1);
> + nread = __read_nocancel (fd, buf, sizeof buf - 1);
> close_not_cancel_no_status (fd);
> if (nread <= 0)
> return -1;
> diff --git a/sysdeps/unix/sysv/linux/getlogin_r.c b/sysdeps/unix/sysv/linux/getlogin_r.c
> index 2519792..1de746b 100644
> --- a/sysdeps/unix/sysv/linux/getlogin_r.c
> +++ b/sysdeps/unix/sysv/linux/getlogin_r.c
> @@ -41,7 +41,7 @@ __getlogin_r_loginuid (char *name, size_t namesize)
> /* We are reading a 32-bit number. 12 bytes are enough for the text
> representation. If not, something is wrong. */
> char uidbuf[12];
> - ssize_t n = TEMP_FAILURE_RETRY (read_not_cancel (fd, uidbuf,
> + ssize_t n = TEMP_FAILURE_RETRY (__read_nocancel (fd, uidbuf,
> sizeof (uidbuf)));
> close_not_cancel_no_status (fd);
>
> diff --git a/sysdeps/unix/sysv/linux/getsysstats.c b/sysdeps/unix/sysv/linux/getsysstats.c
> index e21a34c..d1400d6 100644
> --- a/sysdeps/unix/sysv/linux/getsysstats.c
> +++ b/sysdeps/unix/sysv/linux/getsysstats.c
> @@ -85,7 +85,7 @@ next_line (int fd, char *const buffer, char **cp, char **re,
> *re = buffer + (*re - *cp);
> *cp = buffer;
>
> - ssize_t n = read_not_cancel (fd, *re, buffer_end - *re);
> + ssize_t n = __read_nocancel (fd, *re, buffer_end - *re);
> if (n < 0)
> return NULL;
>
> @@ -96,7 +96,7 @@ next_line (int fd, char *const buffer, char **cp, char **re,
> {
> /* Truncate too long lines. */
> *re = buffer + 3 * (buffer_end - buffer) / 4;
> - n = read_not_cancel (fd, *re, buffer_end - *re);
> + n = __read_nocancel (fd, *re, buffer_end - *re);
> if (n < 0)
> return NULL;
>
> diff --git a/sysdeps/unix/sysv/linux/i386/smp.h b/sysdeps/unix/sysv/linux/i386/smp.h
> index c24f2fd..eb1ac5d 100644
> --- a/sysdeps/unix/sysv/linux/i386/smp.h
> +++ b/sysdeps/unix/sysv/linux/i386/smp.h
> @@ -43,7 +43,7 @@ is_smp_system (void)
> /* This was not successful. Now try reading the /proc filesystem. */
> int fd = __open_nocancel ("/proc/sys/kernel/version", O_RDONLY);
> if (__builtin_expect (fd, 0) == -1
> - || read_not_cancel (fd, u.buf, sizeof (u.buf)) <= 0)
> + || __read_nocancel (fd, u.buf, sizeof (u.buf)) <= 0)
> /* This also didn't work. We give up and say it's a UP machine. */
> u.buf[0] = '\0';
>
> diff --git a/sysdeps/unix/sysv/linux/ia64/has_cpuclock.c b/sysdeps/unix/sysv/linux/ia64/has_cpuclock.c
> index 04e395b..6aab1e8 100644
> --- a/sysdeps/unix/sysv/linux/ia64/has_cpuclock.c
> +++ b/sysdeps/unix/sysv/linux/ia64/has_cpuclock.c
> @@ -37,7 +37,7 @@ has_cpuclock (void)
> /* We expect the file to contain a single digit followed by
> a newline. If the format changes we better not rely on
> the file content. */
> - if (read_not_cancel (fd, buf, sizeof buf) != 2
> + if (__read_nocancel (fd, buf, sizeof buf) != 2
> || buf[0] != '0' || buf[1] != '\n')
> newval = -1;
>
> diff --git a/sysdeps/unix/sysv/linux/libc_fatal.c b/sysdeps/unix/sysv/linux/libc_fatal.c
> index 5b484cf..ca838a7 100644
> --- a/sysdeps/unix/sysv/linux/libc_fatal.c
> +++ b/sysdeps/unix/sysv/linux/libc_fatal.c
> @@ -56,7 +56,7 @@ backtrace_and_maps (int do_abort, bool written, int fd)
> int fd2 = __open_nocancel ("/proc/self/maps", O_RDONLY);
> char buf[1024];
> ssize_t n2;
> - while ((n2 = read_not_cancel (fd2, buf, sizeof (buf))) > 0)
> + while ((n2 = __read_nocancel (fd2, buf, sizeof (buf))) > 0)
> if (write_not_cancel (fd, buf, n2) != n2)
> break;
> close_not_cancel_no_status (fd2);
> diff --git a/sysdeps/unix/sysv/linux/malloc-sysdep.h b/sysdeps/unix/sysv/linux/malloc-sysdep.h
> index cb87b58..7a7acba 100644
> --- a/sysdeps/unix/sysv/linux/malloc-sysdep.h
> +++ b/sysdeps/unix/sysv/linux/malloc-sysdep.h
> @@ -47,7 +47,7 @@ check_may_shrink_heap (void)
> if (fd >= 0)
> {
> char val;
> - ssize_t n = read_not_cancel (fd, &val, 1);
> + ssize_t n = __read_nocancel (fd, &val, 1);
> may_shrink_heap = n > 0 && val == '2';
> close_not_cancel_no_status (fd);
> }
> diff --git a/sysdeps/unix/sysv/linux/not-cancel.h b/sysdeps/unix/sysv/linux/not-cancel.h
> index 8d897f0..3d26075 100644
> --- a/sysdeps/unix/sysv/linux/not-cancel.h
> +++ b/sysdeps/unix/sysv/linux/not-cancel.h
> @@ -34,9 +34,9 @@ libc_hidden_proto (__open_nocancel)
> __typeof (open64) __open64_nocancel;
> libc_hidden_proto (__open64_nocancel)
>
> -/* Uncancelable read. */
> -#define __read_nocancel(fd, buf, len) \
> - INLINE_SYSCALL (read, 3, fd, buf, len)
> +/* Non cancellable read syscall. */
> +__typeof (__read) __read_nocancel;
> +libc_hidden_proto (__read_nocancel)
>
> /* Uncancelable write. */
> #define __write_nocancel(fd, buf, len) \
> @@ -61,10 +61,6 @@ libc_hidden_proto (__open64_nocancel)
> (void) ({ INTERNAL_SYSCALL_DECL (err); \
> INTERNAL_SYSCALL (close, err, 1, (fd)); })
>
> -/* Uncancelable read. */
> -#define read_not_cancel(fd, buf, n) \
> - __read_nocancel (fd, buf, n)
> -
> /* Uncancelable write. */
> #define write_not_cancel(fd, buf, n) \
> __write_nocancel (fd, buf, n)
> diff --git a/sysdeps/unix/sysv/linux/pthread_getname.c b/sysdeps/unix/sysv/linux/pthread_getname.c
> index caab2cc..93c1dfd 100644
> --- a/sysdeps/unix/sysv/linux/pthread_getname.c
> +++ b/sysdeps/unix/sysv/linux/pthread_getname.c
> @@ -50,7 +50,7 @@ pthread_getname_np (pthread_t th, char *buf, size_t len)
> return errno;
>
> int res = 0;
> - ssize_t n = TEMP_FAILURE_RETRY (read_not_cancel (fd, buf, len));
> + ssize_t n = TEMP_FAILURE_RETRY (__read_nocancel (fd, buf, len));
> if (n < 0)
> res = errno;
> else
> diff --git a/sysdeps/unix/sysv/linux/read.c b/sysdeps/unix/sysv/linux/read.c
> index 2a02c1b..e021df8 100644
> --- a/sysdeps/unix/sysv/linux/read.c
> +++ b/sysdeps/unix/sysv/linux/read.c
> @@ -18,6 +18,7 @@
>
> #include <unistd.h>
> #include <sysdep-cancel.h>
> +#include <not-cancel.h>
>
> /* Read NBYTES into BUF from FD. Return the number read or -1. */
> ssize_t
> @@ -31,3 +32,14 @@ libc_hidden_def (__read)
> weak_alias (__libc_read, __read)
> libc_hidden_def (read)
> weak_alias (__libc_read, read)
> +
> +#if !IS_IN (rtld)
> +ssize_t
> +__read_nocancel (int fd, void *buf, size_t nbytes)
> +{
> + return INLINE_SYSCALL_CALL (read, fd, buf, nbytes);
> +}
> +#else
> +strong_alias (__libc_read, __read_nocancel)
> +#endif
> +libc_hidden_def (__read_nocancel)
> diff --git a/sysdeps/unix/sysv/linux/sysconf.c b/sysdeps/unix/sysv/linux/sysconf.c
> index ab068ab..f9becfb 100644
> --- a/sysdeps/unix/sysv/linux/sysconf.c
> +++ b/sysdeps/unix/sysv/linux/sysconf.c
> @@ -121,7 +121,7 @@ __sysconf (int name)
> /* This is more than enough, the file contains a single integer. */
> char buf[32];
> ssize_t n;
> - n = TEMP_FAILURE_RETRY (read_not_cancel (fd, buf, sizeof (buf) - 1));
> + n = TEMP_FAILURE_RETRY (__read_nocancel (fd, buf, sizeof (buf) - 1));
> close_not_cancel_no_status (fd);
>
> if (n > 0)
>