Re: [patch] libstdc++/67747 Allocate space for dirent::d_name

[Martin Sebor <msebor at gmail dot com>]

On 09/29/2015 05:37 AM, Jonathan Wakely wrote:
> POSIX says that dirent::d_name has an unspecified length, so calls to
> readdir_r must pass a buffer with enough trailing space for
> {NAME_MAX}+1 characters. I wasn't doing that, which works OK on
> GNU/Linux and BSD where d_name is a large array, but fails on Solaris
> 32-bit.
>
> This uses pathconf to get NAME_MAX and allocates a buffer.
>
> Tested powerpc64le-linux and x86_64-dragonfly4.1, I'm going to commit
> this to trunk today (and backport all the filesystem fixes to
> gcc-5-branch).

Calling pathconf is only necessary when _POSIX_NO_TRUNC is zero
which I think exists mainly for legacy file systems. Otherwise,
it's safe to use NAME_MAX instead. Avoiding the call to pathconf
also avoids the TOCTOU between it and the call to opendir, and
hardcoding the value makes it possible to avoid dynamically
allocating the dirent buffer.

I didn't remember the MAX_PATH value on Windows anymore but from
what I've just read online it sounds like it's defined to 260.

Defaulting to 255 on POSIX is appropriate. On XSI systems, the
minimum required value is _XOPEN_NAME_MAX which is 255 (I would
suggest using the macro instead when it's defined). Otherwise,
the strictly conforming minimum value would be 14 -- the value
of _POSIX_NAME_MAX, but since 255 is greater it's fine.

Other than that, I tend to be leery of using plain char arrays
as buffers for objects of bigger types. I don't know to what
extent this is a problem for libstdc++ anymore as more and more
hardware is tolerant of misaligned accesses and as the default
new expression typically returns memory suitably aligned for
the largest fundamental type. But since there is no requirement
in the language that it do so and I would tend to err on the
side of caution and use operator new (as opposed to
new char[len]).

Martin

PS I'm interpreting _POSIX_NO_TRUNC being zero as more
restrictive than if it was non-zero and so calling pathconf(p,
_PC_NO_TRUNC) should be required to also return non-zero for
such an implementation, regardless of p. But let me check that
I'm reading it right.