Re: [PATCH 1/4] Sync getcwd with gnulib

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On Thu, Aug 27, 2020 at 5:14 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * Adhemerval Zanella via Libc-alpha:
>
> > +#if HAVE_MINIMALLY_WORKING_GETCWD
> > +  /* If AT_FDCWD is not defined, the algorithm below is O(N**2) and
> > +     this is much slower than the system getcwd (at least on
> > +     GNU/Linux).  So trust the system getcwd's results unless they
> > +     look suspicious.
> > +
> > +     Use the system getcwd even if we have openat support, since the
> > +     system getcwd works even when a parent is unreadable, while the
> > +     openat-based approach does not.
> > +
> > +     But on AIX 5.1..7.1, the system getcwd is not even minimally
> > +     working: If the current directory name is slightly longer than
> > +     PATH_MAX, it omits the first directory component and returns
> > +     this wrong result with errno = 0.  */
> > +
> > +# undef getcwd
> > +  dir = getcwd_system (buf, size);
> > +  if (dir || (size && errno == ERANGE))
> > +    return dir;
>
> This conflicts with the getcwd_system implementation does not set errno.
>
> > +  /* Solaris getcwd (NULL, 0) fails with errno == EINVAL, but it has
> > +     internal magic that lets it work even if an ancestor directory is
> > +     inaccessible, which is better in many cases.  So in this case try
> > +     again with a buffer that's almost always big enough.  */
> > +  if (errno == EINVAL && buf == NULL && size == 0)
> > +    {
> > +      char big_buffer[BIG_FILE_NAME_LENGTH + 1];
> > +      dir = getcwd_system (big_buffer, sizeof big_buffer);
> > +      if (dir)
> > +        return strdup (dir);
> > +    }
>
> > +          /* When we've iterated through all directory entries without finding
> > +             one with a matching d_ino, rewind the stream and consider each
> > +             name again, but this time, using lstat.  This is necessary in a
> > +             chroot on at least one system (glibc-2.3.6 + linux 2.6.12), where
> > +             .., ../.., ../../.., etc. all had the same device number, yet the
> > +             d_ino values for entries in / did not match those obtained
> > +             via lstat.  */
> > +          if (d == NULL && errno == 0 && use_d_ino)
> > +            {
> > +              use_d_ino = false;
> > +              __rewinddir (dirstream);
> > +              d = __readdir (dirstream);
> > +            }
>
> I'm not sure if it's worthwhile to have such code in glibc.
>
> The generic getcwd isn't used by Hurd, right?  Would it make sense to
> have a trimmed-down Linux implementation as well?

This specific part is not used by glibc, for LIBC my patch sets
HAVE_MINIMALLY_WORKING_GETCWD
explicit to 0. I just added to sync with gnulib implementation and
make future changes less complex
(and it also checks if gnulib code to disable the LIBC part is working
correctly).