On Mon, 27 Jun 2022 at 14:32, Jonathan Wakely <jwak...@redhat.com> wrote:
>
> On Mon, 27 Jun 2022 at 14:05, Alexandre Oliva <ol...@adacore.com> wrote:
> >
> > On Jun 27, 2022, Alexandre Oliva <ol...@adacore.com> wrote:
> >
> > > It looks like the atp.pathname is missing the nonexistent_path
> > > assigned to variable dir in test_pr99290, so we attempt to open
> > > subdirs thereof as if with openat.
> >
> > This appears to be caused by the early return in fs::_Dir's ctor:
> >
> > _Dir(const fs::path& p, bool skip_permission_denied, bool nofollow,
> > [[maybe_unused]] bool filename_only, error_code& ec)
> > : _Dir_base(p.c_str(), skip_permission_denied, nofollow, ec)
> > {
> > #if _GLIBCXX_HAVE_DIRFD // && 0
> > if (filename_only)
> > return; // Do not store path p when we aren't going to use it.
> > #endif
>
> Yes, this needs a fix. If we don't have openat then we always need a
> full path relative to the CWD, not just a filename relative to a file
> descriptor for the parent directory.
>
> I think we need to store the directory's path if any of dirfd, openat
> or unlinkat is missing.
>
>
> >
> > if (!ec)
> > path = p;
> > }
> >
> > but somehow disabling the early return to force the saving of path
> > appears to break copy(): copy.cc's test01() succeeded without the '&& 0'
> > that I've commented-out above, but started failing to create 'to' in the
> > copy at line copy.cc:54 when I put it in to prevent the early return.
> >
> > Does that make any sense to you?
>
> No, I'll have to debug the test. I thought that not storing the path
> was just an optimization (to avoid parsing, decomposing, and
> allocating a path object that we will never use).
If I add the && 0 (so we don't store the path) and bodge
<bits/c++config.h> so that HAVE_OPENAT and HAVE_UNLINKAT are both
undefined, I do not see any new FAIL in the
filesystem/operations/copy.cc tests. filesystem::copy doesn't even use
recursive_directory_iterator, and the filename_only bool should always
be false for non-recursive directory_iterator, and so the early return
should never happen anyway.
(I do have an uncommitted patch to avoid storing that path in more
cases, including for non-recursive directory iterators, but it still
wouldn't be true for filesystem::copy).
