[PATCH 1/3] <fd_to_filename.h>: Add type safety and port to Hurd
Florian Weimer
fweimer@redhat.com
Sat Feb 15 13:17:00 GMT 2020
* Paul Eggert:
> 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 :-).
But INT_STRLEN_BOUND is 11, right?
>> +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.
The problem is when an application passes an invalid descriptor to some
libc function and that ends up with __fd_to_filename. We should not
make matters worse in that case.
Maybe we should use an assert?
Thanks,
Florian
More information about the Libc-alpha
mailing list