Re: [PATCH 1/3] <fd_to_filename.h>: Add type safety and port to Hurd

[Paul Eggert <eggert@cs.ucla.edu>]

On 2/14/20 10:11 AM, Florian Weimer wrote:
> +  /* A positive int value has at most 10 decimal digits.  */
> +  char buffer[sizeof (FD_TO_FILENAME_PREFIX) + 10];

This should use 'INT_STRLEN_BOUND (int)' rather than '10', where 
INT_STRLEN_BOUND is taken from intprops.h. Then you don't need the 
comment (and the code won't break on future ILP64 platforms :-).

> +char *
> +__fd_to_filename (int descriptor, struct fd_to_filename *storage)
> +{
> +  char *p = mempcpy (storage->buffer, FD_TO_FILENAME_PREFIX,
> +                     strlen (FD_TO_FILENAME_PREFIX));
> +  if (__glibc_likely (descriptor >= 0))
> +    {
> +      if (descriptor < 1000)
> +        p = digit123 (p, descriptor);
> +      else if (descriptor < 1000 * 1000) ...

It's not clear what that "descriptor >= 0" test is doing. I assume that 
a precondition is that DESCRIPTOR is nonnegative; if so, this should be 
mentioned and the test removed. If it's not a precondition, the code 
should do the right thing if DESCRIPTOR is negative (I suppose, return a 
filename that cannot be opened, though it doesn't do that currently) -- 
which should also be documented but I think this'd be overkill.

As Adhemerval writes, there's no need for special cases for powers of 
1000 etc. Something like the following should do (this is a bit shorter 
than Adhemerval's version):

   /* Convert nonnegative DESCRIPTOR to a file name.  */

   char *
   __fd_to_filename (int descriptor, struct fd_to_filename *storage)
   {
     char *p = mempcpy (storage->buffer, FD_TO_FILENAME_PREFIX,
		       strlen (FD_TO_FILENAME_PREFIX) - 1);
     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;
   }

GCC turns those divisions and remainders into the usual 
multiply+shift+add and this should be good enough.