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

[Jonathan Wakely <jwakely at redhat dot com>]

On 16/09/15 16:04 -0600, Martin Sebor wrote:
> > Tested powerpc64le-linux, x86_64-dragonfly4.1 and x86_64-netbsd5.1,
> > do you see any reason not to commit this for now?
>
> I see only a couple of potential problems: a missing test for
> PATH_MAX in the unlikely event it's not defined (or is obscenely

In the current patch _GLIBCXX_USE_REALPATH won't be defined unless:

      #elif _XOPEN_VERSION >= 700 || defined(PATH_MAX)

so if it's defined and _XOPEN_VERSION < 700 then we know PATH_MAX must
be defined (otherwise _GLIBCXX_USE_REALPATH wouldn't be).

> large), and a missing check to avoid infinite loops due to symlinks.

I thought about keeping track of where I'd been while expanding
symlinks, but then realised this will do it:

 if (!exists(pa, ec))
   {
     fail(ENOENT);
     return result;
   }
 // else we can assume no unresolvable symlink loops

If there's a symlink loop then exists(pa) will fail with ELOOP, and we
won't try to resolve it by hand.

And then after each step in the while(!cmpts.empty()) loop I also have
a check for !exists(result, ec), which should even handle the case
where the filesystem changes after the initial exists() call so that a
loop is introduced while we're canonicalising the path.


> > Any improvements such as hardcoding checks for specific versions of
> > Solaris or the BSDs are QoI, and this is only an experimental TS, so I
> > don't want to spend the rest of stage 1 working on one function :-)
>
> Makes sense.
>
> > My main obstacle to writing good tests right now is having some way to
> > create and destroy files safely in the tests. It's hard to test
> > functions like is_symlink() without first creating a symlink in a
> > known location, and also removing it again cleanly so the next
> > testsuite run doesn't fail if the file is already present.
> >
> > One option would be to have libstdc++-v3/testsuite/Makefile create a
> > new sub-directory as a sandbox for filesystem tests, removing it if it
> > already exists. Then the tests can put anything they like in that new
> > dir without fear of trashing the user's files elsewhere on the FS!
>
> I don't know how you feel about Tcl but writing a filesystem.exp
> and adding a new "dg-fs" API would let each test can set up the
> directory structure it needs.

My Tcl is very weak, but if that's the right approach then I can try
that.

Thanks again!