This is the mail archive of the mailing list for the libstdc++ project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] libstdc++/67173 Fix filesystem::canonical for Solaris 10.

It turns out I was wrong about BSD traditionally supporting
realpath(path, NULL), it first appeared in these relatively recent

FreeBSD 9.0
OpenBSD 5.0
NetBSD 6.1

Like Solaris 11, some of them still define _POSIX_VERSION=200112L even
though they support passing NULL,  so we could hardcode the targets
that are known to support it, but we'll still need a fallback for lots
of slightly older targets anyway.

So here's a new implementation of canonical() which only uses
realpath() when this autoconf compile-or-link test passes:

#if _XOPEN_VERSION < 500
#elif _XOPEN_VERSION >= 700 || defined(PATH_MAX)
char *tmp = realpath((const char*)NULL, (char*)NULL);

i.e. I use realpath() for Issue 7, or for Issue 6 if PATH_MAX is

I probably wouldn't code the PATH_MAX test quite the same way.
I would expect it to be mainly Issue 6 implementations that don't
define the macro to want to provide the null extension since there
would otherwise be no safe way to use the function.

I didn't test it at all but I'd be inclined to write the conditional
to look more along these lines:

  #if _XOPEN_VERSION >= 700
    // Issue 7 and better -- null resolved_name means allocate
    char *tmp = realpath (file_name, (char*)NULL);
  #elif _XOPEN_VERSION == 600
     // Issue 6 -- null resolved_name is implementation-defined
  #  if FreeBSD 9.0 or OpenBSD 5.0 or NetBSD 6.1
    char *tmp = realpath (file_name, (char*)NULL);
  #  elif PATH_MAX
    char *tmp = realpath (file_name, malloc (PATH_MAX));
  #  else
  #    error No safe way to call realpath
  #  endif
    // SUSv2 -- null resolved_name is an error
    char *tmp = realpath (file_name, malloc (PATH_MAX));
  #  error realpath not supported or no safe way to call it

Then in filesystem::canonical() if _GLIBCXX_USE_REALPATH is set I use
it, passing NULL for Issue 7, or allocating a buffer of PATH_MAX
otherwise. If realpath isn't supported or fails with ENAMETOOLONG then
I do the path traversal by hand (which can be done entirely using the
std::experimental::filesystem operations).

FWIW, to work across filesystems with different _PC_PATH_MAX, I
suspect the operations might need to use readlinkat. I.e., they
might need to descend into each individual subdirectory to avoid
forming a temporary pathname that's too long for the file system
being traversed, even though both the initial and the final
pathnames are under the limit. (I haven't actually tested this
but I don't see where GLIBC handles this case so it might not
do the right thing either.)

Only passing NULL for Issue 7 is quite conservative. It means we don't
do it for targets that support it as an implementation-defined
extension to Issue 6, which includes Solaris 11, the BSDs and even
older GNU systems (including RHEL6). But that's OK, we have a fallback
now so it means no loss of functionality, just efficiency.  We can
tweak the config later for targets known to handle NULL.

Does the config test actually run? If not, I don't see the point
(it doesn't tell us anything the POSIX feature tests macros don't).
If it did run, it would fail since the first argument is null.

The new testcase is not very thorough. I've run a few more involved
tests that aren't suitable to check in until I figure out a good way
of running filesystem tests that can create/remove arbitrary files and

Yeah, writing good tests is always the hard part. If you need help
I can try to put some together in my spare time.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]