[RFC] linux: use __fd_to_filename helper function instead of snprintf.

  snprintf() is a very big hammer and pulls in a lot of code that might be
unnecessary. It was also being used in an inconsistent manner between
the affected files.

Change made to fchmodat and fexecve. There are tests using xasprintf
instead of this helper as well, but this commit doesn't touch them.
---

I found these call sites using `grep -R 'printf.*proc/self/fd'`, but
there are probably other instances that weren't as simple.

support/support_descriptors.c:        char *path = xasprintf ("/proc/self/fd/%ld", fd);
sysdeps/unix/sysv/linux/tst-ttyname.c:      char *linkname = xasprintf ("/proc/self/fd/%d", slave);
sysdeps/unix/sysv/linux/tst-ttyname.c:          char *linkname = xasprintf ("/proc/self/fd/%d", slave);
sysdeps/unix/sysv/linux/tst-ttyname.c:    char *linkname = xasprintf ("/proc/self/fd/%d", slave);
sysdeps/unix/sysv/linux/tst-memfd_create.c:        char *fd_path = xasprintf ("/proc/self/fd/%d", fd);
io/tst-open-tmpfile.c:  char *proc_fd_path = xasprintf ("/proc/self/fd/%d", fd);
io/tst-open-tmpfile.c:  char *proc_fd_path = xasprintf ("/proc/self/fd/%d", fd);

Some of the tests can be moved to use __fd_to_filename, but I held off
on doing this to hear feedback on the idea first.

It seems not all files follow the same include order / grouping policy,
so I'd appreciate pointers on that as well.

 sysdeps/unix/sysv/linux/fchmodat.c | 11 +++--------
 sysdeps/unix/sysv/linux/fexecve.c  |  5 +++--
 2 files changed, 6 insertions(+), 10 deletions(-)
  

* Érico Nogueira via Libc-alpha:

> diff --git a/sysdeps/unix/sysv/linux/fexecve.c b/sysdeps/unix/sysv/linux/fexecve.c
> index f37c245396..218337a3b5 100644
> --- a/sysdeps/unix/sysv/linux/fexecve.c
> +++ b/sysdeps/unix/sysv/linux/fexecve.c
> @@ -22,6 +22,7 @@
>  #include <fcntl.h>
>  #include <sys/stat.h>
>  
> +#include <fd_to_filename.h>
>  #include <sysdep.h>
>  #include <sys/syscall.h>
>  #include <kernel-features.h>
> @@ -50,8 +51,8 @@ fexecve (int fd, char *const argv[], char *const envp[])
>  #ifndef __ASSUME_EXECVEAT
>    /* We use the /proc filesystem to get the information.  If it is not
>       mounted we fail.  */
> -  char buf[sizeof "/proc/self/fd/" + sizeof (int) * 3];
> -  __snprintf (buf, sizeof (buf), "/proc/self/fd/%d", fd);
> +  struct fd_to_filename filename;
> +  char *buf = fd_to_filename(fd, &filename);

Sorry, this patch fails to build if applied to current master:

../sysdeps/unix/sysv/linux/fexecve.c: In function ‘fexecve’:
../sysdeps/unix/sysv/linux/fexecve.c:55:15: error: implicit declaration of function ‘fd_to_filename’ [-Werror=implicit-function-declaration]
   55 |   char *buf = fd_to_filename(fd, &filename);
      |               ^~~~~~~~~~~~~~

Please also add a space before the opening '(', to match GNU style, and
consider if the code becomes clearer if you avoid the buf variable.

Thanks,
Florian
  

Em 27/04/2021 03:46, Florian Weimer escreveu:
> * Érico Nogueira via Libc-alpha:
> 
>> diff --git a/sysdeps/unix/sysv/linux/fexecve.c b/sysdeps/unix/sysv/linux/fexecve.c
>> index f37c245396..218337a3b5 100644
>> --- a/sysdeps/unix/sysv/linux/fexecve.c
>> +++ b/sysdeps/unix/sysv/linux/fexecve.c
>> @@ -22,6 +22,7 @@
>>   #include <fcntl.h>
>>   #include <sys/stat.h>
>>   
>> +#include <fd_to_filename.h>
>>   #include <sysdep.h>
>>   #include <sys/syscall.h>
>>   #include <kernel-features.h>
>> @@ -50,8 +51,8 @@ fexecve (int fd, char *const argv[], char *const envp[])
>>   #ifndef __ASSUME_EXECVEAT
>>     /* We use the /proc filesystem to get the information.  If it is not
>>        mounted we fail.  */
>> -  char buf[sizeof "/proc/self/fd/" + sizeof (int) * 3];
>> -  __snprintf (buf, sizeof (buf), "/proc/self/fd/%d", fd);
>> +  struct fd_to_filename filename;
>> +  char *buf = fd_to_filename(fd, &filename);
> 
> Sorry, this patch fails to build if applied to current master:

I hadn't commited the fix from my work tree, sorry.

> 
> ../sysdeps/unix/sysv/linux/fexecve.c: In function ‘fexecve’:
> ../sysdeps/unix/sysv/linux/fexecve.c:55:15: error: implicit declaration of function ‘fd_to_filename’ [-Werror=implicit-function-declaration]
>     55 |   char *buf = fd_to_filename(fd, &filename);
>        |               ^~~~~~~~~~~~~~
> 
> Please also add a space before the opening '(', to match GNU style, and
> consider if the code becomes clearer if you avoid the buf variable.

I think it's basically as clear as, but a bit more compact, so I will 
use that instead. Thanks!

> 
> Thanks,
> Florian
>