* [PATCH 1/3] misc: use _fitoa_word to implement __fd_to_filename. @ 2021-05-04 1:51 Érico Nogueira 2021-05-04 1:51 ` [PATCH 2/3] linux: use fd_to_filename instead of _fitoa_word in ttyname_r Érico Nogueira ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Érico Nogueira @ 2021-05-04 1:51 UTC (permalink / raw) To: libc-alpha; +Cc: Érico Nogueira In a default build for x86_64, size decreased by 24 bytes: 1883294 to 1883270. Aditionally, avoids repeating the number printing logic in multiple places. --- misc/fd_to_filename.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/misc/fd_to_filename.c b/misc/fd_to_filename.c index 86a1a1acc2..20c2014903 100644 --- a/misc/fd_to_filename.c +++ b/misc/fd_to_filename.c @@ -20,6 +20,7 @@ #include <assert.h> #include <string.h> +#include <_itoa.h> char * __fd_to_filename (int descriptor, struct fd_to_filename *storage) @@ -28,11 +29,7 @@ __fd_to_filename (int descriptor, struct fd_to_filename *storage) char *p = mempcpy (storage->buffer, FD_TO_FILENAME_PREFIX, strlen (FD_TO_FILENAME_PREFIX)); + *_fitoa_word (descriptor, p, 10, 0) = '\0'; - for (int d = descriptor; p++, (d /= 10) != 0; ) - continue; - *p = '\0'; - for (int d = descriptor; *--p = '0' + d % 10, (d /= 10) != 0; ) - continue; return storage->buffer; } -- 2.31.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] linux: use fd_to_filename instead of _fitoa_word in ttyname_r. 2021-05-04 1:51 [PATCH 1/3] misc: use _fitoa_word to implement __fd_to_filename Érico Nogueira @ 2021-05-04 1:51 ` Érico Nogueira 2021-05-04 13:18 ` Adhemerval Zanella 2021-05-04 1:51 ` [PATCH 3/3] linux: implement ttyname as a wrapper around ttyname_r Érico Nogueira 2021-05-04 13:18 ` [PATCH 1/3] misc: use _fitoa_word to implement __fd_to_filename Adhemerval Zanella 2 siblings, 1 reply; 9+ messages in thread From: Érico Nogueira @ 2021-05-04 1:51 UTC (permalink / raw) To: libc-alpha; +Cc: Érico Nogueira Simplifies the logic and makes intent clearer, while at the same time decreasing binary size. On x86_64, dropped from 1883270 to 1883206, a 64 byte decrease. --- sysdeps/unix/sysv/linux/ttyname_r.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/sysdeps/unix/sysv/linux/ttyname_r.c b/sysdeps/unix/sysv/linux/ttyname_r.c index c1092d1695..fa1578fb64 100644 --- a/sysdeps/unix/sysv/linux/ttyname_r.c +++ b/sysdeps/unix/sysv/linux/ttyname_r.c @@ -26,7 +26,7 @@ #include <string.h> #include <stdlib.h> -#include <_itoa.h> +#include <fd_to_filename.h> #include "ttyname.h" @@ -92,7 +92,7 @@ getttyname_r (char *buf, size_t buflen, const struct stat64 *mytty, int __ttyname_r (int fd, char *buf, size_t buflen) { - char procname[30]; + struct fd_to_filename filename; struct stat64 st, st1; int dostat = 0; int doispty = 0; @@ -122,9 +122,7 @@ __ttyname_r (int fd, char *buf, size_t buflen) return errno; /* We try using the /proc filesystem. */ - *_fitoa_word (fd, __stpcpy (procname, "/proc/self/fd/"), 10, 0) = '\0'; - - ssize_t ret = __readlink (procname, buf, buflen - 1); + ssize_t ret = __readlink (__fd_to_filename (fd, &filename), buf, buflen - 1); if (__glibc_unlikely (ret == -1 && errno == ENAMETOOLONG)) { __set_errno (ERANGE); -- 2.31.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] linux: use fd_to_filename instead of _fitoa_word in ttyname_r. 2021-05-04 1:51 ` [PATCH 2/3] linux: use fd_to_filename instead of _fitoa_word in ttyname_r Érico Nogueira @ 2021-05-04 13:18 ` Adhemerval Zanella 0 siblings, 0 replies; 9+ messages in thread From: Adhemerval Zanella @ 2021-05-04 13:18 UTC (permalink / raw) To: libc-alpha On 03/05/2021 22:51, Érico Nogueira via Libc-alpha wrote: > Simplifies the logic and makes intent clearer, while at the same time > decreasing binary size. > > On x86_64, dropped from 1883270 to 1883206, a 64 byte decrease. LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > sysdeps/unix/sysv/linux/ttyname_r.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/sysdeps/unix/sysv/linux/ttyname_r.c b/sysdeps/unix/sysv/linux/ttyname_r.c > index c1092d1695..fa1578fb64 100644 > --- a/sysdeps/unix/sysv/linux/ttyname_r.c > +++ b/sysdeps/unix/sysv/linux/ttyname_r.c > @@ -26,7 +26,7 @@ > #include <string.h> > #include <stdlib.h> > > -#include <_itoa.h> > +#include <fd_to_filename.h> > > #include "ttyname.h" > > @@ -92,7 +92,7 @@ getttyname_r (char *buf, size_t buflen, const struct stat64 *mytty, > int > __ttyname_r (int fd, char *buf, size_t buflen) > { > - char procname[30]; > + struct fd_to_filename filename; > struct stat64 st, st1; > int dostat = 0; > int doispty = 0; > @@ -122,9 +122,7 @@ __ttyname_r (int fd, char *buf, size_t buflen) > return errno; > > /* We try using the /proc filesystem. */ > - *_fitoa_word (fd, __stpcpy (procname, "/proc/self/fd/"), 10, 0) = '\0'; > - > - ssize_t ret = __readlink (procname, buf, buflen - 1); > + ssize_t ret = __readlink (__fd_to_filename (fd, &filename), buf, buflen - 1); > if (__glibc_unlikely (ret == -1 && errno == ENAMETOOLONG)) > { > __set_errno (ERANGE); > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] linux: implement ttyname as a wrapper around ttyname_r. 2021-05-04 1:51 [PATCH 1/3] misc: use _fitoa_word to implement __fd_to_filename Érico Nogueira 2021-05-04 1:51 ` [PATCH 2/3] linux: use fd_to_filename instead of _fitoa_word in ttyname_r Érico Nogueira @ 2021-05-04 1:51 ` Érico Nogueira 2021-05-04 16:37 ` Adhemerval Zanella 2021-05-04 13:18 ` [PATCH 1/3] misc: use _fitoa_word to implement __fd_to_filename Adhemerval Zanella 2 siblings, 1 reply; 9+ messages in thread From: Érico Nogueira @ 2021-05-04 1:51 UTC (permalink / raw) To: libc-alpha; +Cc: Érico Nogueira Big win in binary size and avoids duplicating the logic in multiple places. On x86_64, dropped from 1883206 to 1881790, a 1416 byte decrease. Also changed logic to track if ttyname_buf has been allocated by checking if it's NULL instead of tracking buflen as an additional variable. --- I was going to simply change the function to use __fd_to_filename, like the 2/3 patch, but this felt like a better improvement. However, I haven't studied the implications deeply or verified if the ttyname() logic behaved noticeably (or subtly) different than what ttyname_r() did. sysdeps/unix/sysv/linux/ttyname.c | 161 ++---------------------------- 1 file changed, 9 insertions(+), 152 deletions(-) diff --git a/sysdeps/unix/sysv/linux/ttyname.c b/sysdeps/unix/sysv/linux/ttyname.c index d561ac12f2..a2bb81c78b 100644 --- a/sysdeps/unix/sysv/linux/ttyname.c +++ b/sysdeps/unix/sysv/linux/ttyname.c @@ -17,17 +17,9 @@ #include <errno.h> #include <limits.h> -#include <stddef.h> -#include <dirent.h> -#include <sys/types.h> -#include <sys/stat.h> #include <termios.h> -#include <unistd.h> -#include <string.h> #include <stdlib.h> -#include <_itoa.h> - #include "ttyname.h" #if 0 @@ -35,170 +27,35 @@ char *__ttyname; #endif -static char *getttyname (const char *dev, const struct stat64 *mytty, - int save, int *dostat); - -libc_freeres_ptr (static char *getttyname_name); - -static char * -attribute_compat_text_section -getttyname (const char *dev, const struct stat64 *mytty, int save, int *dostat) -{ - static size_t namelen; - struct stat64 st; - DIR *dirstream; - struct dirent64 *d; - size_t devlen = strlen (dev) + 1; - - dirstream = __opendir (dev); - if (dirstream == NULL) - { - *dostat = -1; - return NULL; - } - - /* Prepare for the loop. If we already have a buffer copy the directory - name we look at into it. */ - if (devlen < namelen) - *((char *) __mempcpy (getttyname_name, dev, devlen - 1)) = '/'; - - while ((d = __readdir64 (dirstream)) != NULL) - if ((d->d_fileno == mytty->st_ino || *dostat) - && strcmp (d->d_name, "stdin") - && strcmp (d->d_name, "stdout") - && strcmp (d->d_name, "stderr")) - { - size_t dlen = _D_ALLOC_NAMLEN (d); - if (devlen + dlen > namelen) - { - free (getttyname_name); - namelen = 2 * (devlen + dlen); /* Big enough. */ - getttyname_name = malloc (namelen); - if (! getttyname_name) - { - *dostat = -1; - /* Perhaps it helps to free the directory stream buffer. */ - (void) __closedir (dirstream); - return NULL; - } - *((char *) __mempcpy (getttyname_name, dev, devlen - 1)) = '/'; - } - memcpy (&getttyname_name[devlen], d->d_name, dlen); - if (__stat64 (getttyname_name, &st) == 0 - && is_mytty (mytty, &st)) - { - (void) __closedir (dirstream); -#if 0 - __ttyname = getttyname_name; -#endif - __set_errno (save); - return getttyname_name; - } - } - - (void) __closedir (dirstream); - __set_errno (save); - return NULL; -} - - +#define TTYNAME_BUFLEN (PATH_MAX-1) /* Static buffer in `ttyname'. */ libc_freeres_ptr (static char *ttyname_buf); - /* Return the pathname of the terminal FD is open on, or NULL on errors. The returned storage is good only until the next call to this function. */ char * ttyname (int fd) { - static size_t buflen; - char procname[30]; - struct stat64 st, st1; - int dostat = 0; - int doispty = 0; - char *name; - int save = errno; - struct termios term; - /* isatty check, tcgetattr is used because it sets the correct - errno (EBADF resp. ENOTTY) on error. */ + errno (EBADF resp. ENOTTY) on error. Fast error path to avoid the allocation */ + struct termios term; if (__glibc_unlikely (__tcgetattr (fd, &term) < 0)) return NULL; - if (__fstat64 (fd, &st) < 0) - return NULL; - - /* We try using the /proc filesystem. */ - *_fitoa_word (fd, __stpcpy (procname, "/proc/self/fd/"), 10, 0) = '\0'; - - if (buflen == 0) + if (ttyname_buf == NULL) { - buflen = 4095; - ttyname_buf = (char *) malloc (buflen + 1); + ttyname_buf = malloc (TTYNAME_BUFLEN + 1); if (ttyname_buf == NULL) { - buflen = 0; return NULL; } } - ssize_t len = __readlink (procname, ttyname_buf, buflen); - if (__glibc_likely (len != -1)) - { - if ((size_t) len >= buflen) - return NULL; - -#define UNREACHABLE_LEN strlen ("(unreachable)") - if (len > UNREACHABLE_LEN - && memcmp (ttyname_buf, "(unreachable)", UNREACHABLE_LEN) == 0) - { - memmove (ttyname_buf, ttyname_buf + UNREACHABLE_LEN, - len - UNREACHABLE_LEN); - len -= UNREACHABLE_LEN; - } - - /* readlink need not terminate the string. */ - ttyname_buf[len] = '\0'; - - /* Verify readlink result, fall back on iterating through devices. */ - if (ttyname_buf[0] == '/' - && __stat64 (ttyname_buf, &st1) == 0 - && is_mytty (&st, &st1)) - return ttyname_buf; - - doispty = 1; - } - - if (__stat64 ("/dev/pts", &st1) == 0 && S_ISDIR (st1.st_mode)) - { - name = getttyname ("/dev/pts", &st, save, &dostat); - } - else - { - __set_errno (save); - name = NULL; - } - - if (!name && dostat != -1) + int result = ttyname_r (fd, ttyname_buf, TTYNAME_BUFLEN); + if (result != 0) { - name = getttyname ("/dev", &st, save, &dostat); - } - - if (!name && dostat != -1) - { - dostat = 1; - name = getttyname ("/dev", &st, save, &dostat); - } - - if (!name && doispty && is_pty (&st)) - { - /* We failed to figure out the TTY's name, but we can at least - signal that we did verify that it really is a PTY slave. - This happens when we have inherited the file descriptor from - a different mount namespace. */ - __set_errno (ENODEV); + __set_errno (result); return NULL; } - - return name; + return ttyname_buf; } -- 2.31.1 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] linux: implement ttyname as a wrapper around ttyname_r. 2021-05-04 1:51 ` [PATCH 3/3] linux: implement ttyname as a wrapper around ttyname_r Érico Nogueira @ 2021-05-04 16:37 ` Adhemerval Zanella 2021-05-04 16:43 ` Adhemerval Zanella 0 siblings, 1 reply; 9+ messages in thread From: Adhemerval Zanella @ 2021-05-04 16:37 UTC (permalink / raw) To: libc-alpha, Érico Nogueira On 03/05/2021 22:51, Érico Nogueira via Libc-alpha wrote: > Big win in binary size and avoids duplicating the logic in multiple > places. > > On x86_64, dropped from 1883206 to 1881790, a 1416 byte decrease. > > Also changed logic to track if ttyname_buf has been allocated by > checking if it's NULL instead of tracking buflen as an additional > variable. > --- > > I was going to simply change the function to use __fd_to_filename, like > the 2/3 patch, but this felt like a better improvement. However, I > haven't studied the implications deeply or verified if the ttyname() > logic behaved noticeably (or subtly) different than what ttyname_r() > did. > > sysdeps/unix/sysv/linux/ttyname.c | 161 ++---------------------------- > 1 file changed, 9 insertions(+), 152 deletions(-) > > diff --git a/sysdeps/unix/sysv/linux/ttyname.c b/sysdeps/unix/sysv/linux/ttyname.c > index d561ac12f2..a2bb81c78b 100644 > --- a/sysdeps/unix/sysv/linux/ttyname.c > +++ b/sysdeps/unix/sysv/linux/ttyname.c > @@ -17,17 +17,9 @@ > > #include <errno.h> > #include <limits.h> > -#include <stddef.h> > -#include <dirent.h> > -#include <sys/types.h> > -#include <sys/stat.h> > #include <termios.h> > -#include <unistd.h> > -#include <string.h> > #include <stdlib.h> > > -#include <_itoa.h> > - > #include "ttyname.h" > > #if 0 > @@ -35,170 +27,35 @@ > char *__ttyname; > #endif Maybe remove this dead code as well. > > -static char *getttyname (const char *dev, const struct stat64 *mytty, > - int save, int *dostat); > - > -libc_freeres_ptr (static char *getttyname_name); > - > -static char * > -attribute_compat_text_section > -getttyname (const char *dev, const struct stat64 *mytty, int save, int *dostat) > -{ > - static size_t namelen; > - struct stat64 st; > - DIR *dirstream; > - struct dirent64 *d; > - size_t devlen = strlen (dev) + 1; > - > - dirstream = __opendir (dev); > - if (dirstream == NULL) > - { > - *dostat = -1; > - return NULL; > - } > - > - /* Prepare for the loop. If we already have a buffer copy the directory > - name we look at into it. */ > - if (devlen < namelen) > - *((char *) __mempcpy (getttyname_name, dev, devlen - 1)) = '/'; > - > - while ((d = __readdir64 (dirstream)) != NULL) > - if ((d->d_fileno == mytty->st_ino || *dostat) > - && strcmp (d->d_name, "stdin") > - && strcmp (d->d_name, "stdout") > - && strcmp (d->d_name, "stderr")) > - { > - size_t dlen = _D_ALLOC_NAMLEN (d); > - if (devlen + dlen > namelen) > - { > - free (getttyname_name); > - namelen = 2 * (devlen + dlen); /* Big enough. */ > - getttyname_name = malloc (namelen); > - if (! getttyname_name) > - { > - *dostat = -1; > - /* Perhaps it helps to free the directory stream buffer. */ > - (void) __closedir (dirstream); > - return NULL; > - } > - *((char *) __mempcpy (getttyname_name, dev, devlen - 1)) = '/'; > - } > - memcpy (&getttyname_name[devlen], d->d_name, dlen); > - if (__stat64 (getttyname_name, &st) == 0 > - && is_mytty (mytty, &st)) > - { > - (void) __closedir (dirstream); > -#if 0 > - __ttyname = getttyname_name; > -#endif > - __set_errno (save); > - return getttyname_name; > - } > - } > - > - (void) __closedir (dirstream); > - __set_errno (save); > - return NULL; > -} > - > - > +#define TTYNAME_BUFLEN (PATH_MAX-1) > /* Static buffer in `ttyname'. */ > libc_freeres_ptr (static char *ttyname_buf); Although not really related to this change, since you are touching it I think it should use a libc_freeres_fn instead: | static char *ttyname_buf; | | libc_freeres_fn (free_mem) | { | free (ttyname_buf); | } > > - > /* Return the pathname of the terminal FD is open on, or NULL on errors. > The returned storage is good only until the next call to this function. */ > char * > ttyname (int fd) > { > - static size_t buflen; > - char procname[30]; > - struct stat64 st, st1; > - int dostat = 0; > - int doispty = 0; > - char *name; > - int save = errno; > - struct termios term; > - > /* isatty check, tcgetattr is used because it sets the correct > - errno (EBADF resp. ENOTTY) on error. */ > + errno (EBADF resp. ENOTTY) on error. Fast error path to avoid the allocation */ Double space after period and line too long. > + struct termios term; > if (__glibc_unlikely (__tcgetattr (fd, &term) < 0)) > return NULL; > > - if (__fstat64 (fd, &st) < 0) > - return NULL; > - > - /* We try using the /proc filesystem. */ > - *_fitoa_word (fd, __stpcpy (procname, "/proc/self/fd/"), 10, 0) = '\0'; > - > - if (buflen == 0) > + if (ttyname_buf == NULL) > { > - buflen = 4095; > - ttyname_buf = (char *) malloc (buflen + 1); > + ttyname_buf = malloc (TTYNAME_BUFLEN + 1); > if (ttyname_buf == NULL) > { > - buflen = 0; > return NULL; > } > } > Ok. > - ssize_t len = __readlink (procname, ttyname_buf, buflen); > - if (__glibc_likely (len != -1)) > - { > - if ((size_t) len >= buflen) > - return NULL; > - > -#define UNREACHABLE_LEN strlen ("(unreachable)") > - if (len > UNREACHABLE_LEN > - && memcmp (ttyname_buf, "(unreachable)", UNREACHABLE_LEN) == 0) > - { > - memmove (ttyname_buf, ttyname_buf + UNREACHABLE_LEN, > - len - UNREACHABLE_LEN); > - len -= UNREACHABLE_LEN; > - } > - > - /* readlink need not terminate the string. */ > - ttyname_buf[len] = '\0'; > - > - /* Verify readlink result, fall back on iterating through devices. */ > - if (ttyname_buf[0] == '/' > - && __stat64 (ttyname_buf, &st1) == 0 > - && is_mytty (&st, &st1)) > - return ttyname_buf; > - > - doispty = 1; > - } > - > - if (__stat64 ("/dev/pts", &st1) == 0 && S_ISDIR (st1.st_mode)) > - { > - name = getttyname ("/dev/pts", &st, save, &dostat); > - } > - else > - { > - __set_errno (save); > - name = NULL; > - } > - > - if (!name && dostat != -1) > + int result = ttyname_r (fd, ttyname_buf, TTYNAME_BUFLEN); > + if (result != 0) Why do you need to pass the allocated buffer size minus 1? The ttyname_r should handle it, it already pass buflen - 1 on readlink for instance. > { > - name = getttyname ("/dev", &st, save, &dostat); > - } > - > - if (!name && dostat != -1) > - { > - dostat = 1; > - name = getttyname ("/dev", &st, save, &dostat); > - } > - > - if (!name && doispty && is_pty (&st)) > - { > - /* We failed to figure out the TTY's name, but we can at least > - signal that we did verify that it really is a PTY slave. > - This happens when we have inherited the file descriptor from > - a different mount namespace. */ > - __set_errno (ENODEV); > + __set_errno (result); > return NULL; > } > - > - return name; > + return ttyname_buf; > } > With this change ttyname now returns ERANGE instead of ENAMETOOLONG if readlink returns a link larger than the specified buffer, but I think it should be ok. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] linux: implement ttyname as a wrapper around ttyname_r. 2021-05-04 16:37 ` Adhemerval Zanella @ 2021-05-04 16:43 ` Adhemerval Zanella 2021-05-07 17:21 ` Adhemerval Zanella 0 siblings, 1 reply; 9+ messages in thread From: Adhemerval Zanella @ 2021-05-04 16:43 UTC (permalink / raw) To: libc-alpha, Érico Nogueira On 04/05/2021 13:37, Adhemerval Zanella wrote: >> - >> - if (!name && dostat != -1) >> + int result = ttyname_r (fd, ttyname_buf, TTYNAME_BUFLEN); >> + if (result != 0) > > Why do you need to pass the allocated buffer size minus 1? The ttyname_r > should handle it, it already pass buflen - 1 on readlink for instance. You also need a libc_hidden_{proto,def} ttyname_r to avoid the intra PLT (the elf/check-localplt should have warned you about it). ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] linux: implement ttyname as a wrapper around ttyname_r. 2021-05-04 16:43 ` Adhemerval Zanella @ 2021-05-07 17:21 ` Adhemerval Zanella 2021-05-07 19:20 ` Érico Nogueira 0 siblings, 1 reply; 9+ messages in thread From: Adhemerval Zanella @ 2021-05-07 17:21 UTC (permalink / raw) To: libc-alpha, Érico Nogueira On 04/05/2021 13:43, Adhemerval Zanella wrote: > > > On 04/05/2021 13:37, Adhemerval Zanella wrote: >>> - >>> - if (!name && dostat != -1) >>> + int result = ttyname_r (fd, ttyname_buf, TTYNAME_BUFLEN); >>> + if (result != 0) >> >> Why do you need to pass the allocated buffer size minus 1? The ttyname_r >> should handle it, it already pass buflen - 1 on readlink for instance. > > You also need a libc_hidden_{proto,def} ttyname_r to avoid the intra > PLT (the elf/check-localplt should have warned you about it). > I fixed my previous noted issues along with this one and push it upstream, thanks for the patch. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] linux: implement ttyname as a wrapper around ttyname_r. 2021-05-07 17:21 ` Adhemerval Zanella @ 2021-05-07 19:20 ` Érico Nogueira 0 siblings, 0 replies; 9+ messages in thread From: Érico Nogueira @ 2021-05-07 19:20 UTC (permalink / raw) To: Adhemerval Zanella, libc-alpha On Fri May 7, 2021 at 2:21 PM -03, Adhemerval Zanella wrote: > > > On 04/05/2021 13:43, Adhemerval Zanella wrote: > > > > > > On 04/05/2021 13:37, Adhemerval Zanella wrote: > >>> - > >>> - if (!name && dostat != -1) > >>> + int result = ttyname_r (fd, ttyname_buf, TTYNAME_BUFLEN); > >>> + if (result != 0) > >> > >> Why do you need to pass the allocated buffer size minus 1? The ttyname_r > >> should handle it, it already pass buflen - 1 on readlink for instance. > > > > You also need a libc_hidden_{proto,def} ttyname_r to avoid the intra > > PLT (the elf/check-localplt should have warned you about it). > > > > I fixed my previous noted issues along with this one and push it > upstream, thanks for the patch. Ah, great! I hadn't gotten to this yet, thanks a lot for the help and reviews :) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] misc: use _fitoa_word to implement __fd_to_filename. 2021-05-04 1:51 [PATCH 1/3] misc: use _fitoa_word to implement __fd_to_filename Érico Nogueira 2021-05-04 1:51 ` [PATCH 2/3] linux: use fd_to_filename instead of _fitoa_word in ttyname_r Érico Nogueira 2021-05-04 1:51 ` [PATCH 3/3] linux: implement ttyname as a wrapper around ttyname_r Érico Nogueira @ 2021-05-04 13:18 ` Adhemerval Zanella 2 siblings, 0 replies; 9+ messages in thread From: Adhemerval Zanella @ 2021-05-04 13:18 UTC (permalink / raw) To: Érico Nogueira, libc-alpha On 03/05/2021 22:51, Érico Nogueira via Libc-alpha wrote: > In a default build for x86_64, size decreased by 24 bytes: > 1883294 to 1883270. > > Aditionally, avoids repeating the number printing logic in multiple > places. LGTM, thanks. Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org> > --- > misc/fd_to_filename.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/misc/fd_to_filename.c b/misc/fd_to_filename.c > index 86a1a1acc2..20c2014903 100644 > --- a/misc/fd_to_filename.c > +++ b/misc/fd_to_filename.c > @@ -20,6 +20,7 @@ > > #include <assert.h> > #include <string.h> > +#include <_itoa.h> > > char * > __fd_to_filename (int descriptor, struct fd_to_filename *storage) > @@ -28,11 +29,7 @@ __fd_to_filename (int descriptor, struct fd_to_filename *storage) > > char *p = mempcpy (storage->buffer, FD_TO_FILENAME_PREFIX, > strlen (FD_TO_FILENAME_PREFIX)); > + *_fitoa_word (descriptor, p, 10, 0) = '\0'; > > - for (int d = descriptor; p++, (d /= 10) != 0; ) > - continue; > - *p = '\0'; > - for (int d = descriptor; *--p = '0' + d % 10, (d /= 10) != 0; ) > - continue; > return storage->buffer; > } > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-05-07 19:21 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-04 1:51 [PATCH 1/3] misc: use _fitoa_word to implement __fd_to_filename Érico Nogueira 2021-05-04 1:51 ` [PATCH 2/3] linux: use fd_to_filename instead of _fitoa_word in ttyname_r Érico Nogueira 2021-05-04 13:18 ` Adhemerval Zanella 2021-05-04 1:51 ` [PATCH 3/3] linux: implement ttyname as a wrapper around ttyname_r Érico Nogueira 2021-05-04 16:37 ` Adhemerval Zanella 2021-05-04 16:43 ` Adhemerval Zanella 2021-05-07 17:21 ` Adhemerval Zanella 2021-05-07 19:20 ` Érico Nogueira 2021-05-04 13:18 ` [PATCH 1/3] misc: use _fitoa_word to implement __fd_to_filename 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).