posix/glob.c: update from gnulib

posix/glob.c: update from gnulib

[DJ Delorie]

Copied from gnulib/lib/glob.c in order to fix rhbz 1982608

Used config.h instead of libc-config.h

The #ifdef around #define dirfd() was changed to #undef due to
conflicts between glibc's internal and external definitions of
dirfd().  This has been reported to gnulib.

diff --git a/posix/glob.c b/posix/glob.c
index a2b5aabada..19b876f9f0 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -28,6 +28,7 @@
 #include <glob.h>
 
 #include <errno.h>
+#include <fcntl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <stdbool.h>
@@ -56,6 +57,8 @@
 # define sysconf(id) __sysconf (id)
 # define closedir(dir) __closedir (dir)
 # define opendir(name) __opendir (name)
+# undef dirfd
+# define dirfd(str) __dirfd (str)
 # define readdir(str) __readdir64 (str)
 # define getpwnam_r(name, bufp, buf, len, res) \
     __getpwnam_r (name, bufp, buf, len, res)
@@ -69,11 +72,8 @@
 # ifndef GLOB_LSTAT
 #  define GLOB_LSTAT            gl_lstat
 # endif
-# ifndef GLOB_STAT64
-#  define GLOB_STAT64           __stat64
-# endif
-# ifndef GLOB_LSTAT64
-#  define GLOB_LSTAT64          __lstat64
+# ifndef GLOB_FSTATAT64
+#  define GLOB_FSTATAT64        __fstatat64
 # endif
 # include <shlib-compat.h>
 #else /* !_LIBC */
@@ -88,8 +88,7 @@
 # define struct_stat            struct stat
 # define struct_stat64          struct stat
 # define GLOB_LSTAT             gl_lstat
-# define GLOB_STAT64            stat
-# define GLOB_LSTAT64           lstat
+# define GLOB_FSTATAT64         fstatat
 #endif /* _LIBC */
 
 #include <fnmatch.h>
@@ -215,7 +214,8 @@ glob_lstat (glob_t *pglob, int flags, const char *fullname)
   } ust;
   return (__glibc_unlikely (flags & GLOB_ALTDIRFUNC)
           ? pglob->GLOB_LSTAT (fullname, &ust.st)
-          : GLOB_LSTAT64 (fullname, &ust.st64));
+          : GLOB_FSTATAT64 (AT_FDCWD, fullname, &ust.st64,
+                            AT_SYMLINK_NOFOLLOW));
 }
 
 /* Set *R = A + B.  Return true if the answer is mathematically
@@ -257,7 +257,8 @@ is_dir (char const *filename, int flags, glob_t const *pglob)
   struct_stat64 st64;
   return (__glibc_unlikely (flags & GLOB_ALTDIRFUNC)
           ? pglob->gl_stat (filename, &st) == 0 && S_ISDIR (st.st_mode)
-          : GLOB_STAT64 (filename, &st64) == 0 && S_ISDIR (st64.st_mode));
+          : (GLOB_FSTATAT64 (AT_FDCWD, filename, &st64, 0) == 0
+             && S_ISDIR (st64.st_mode)));
 }
 
 /* Find the end of the sub-pattern in a brace expression.  */
@@ -747,6 +748,8 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
       else
         {
 #ifndef WINDOWS32
+          /* Recognize ~user as a shorthand for the specified user's home
+             directory.  */
           char *end_name = strchr (dirname, '/');
           char *user_name;
           int malloc_user_name = 0;
@@ -885,7 +888,22 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
               }
             scratch_buffer_free (&pwtmpbuf);
           }
-#endif /* !WINDOWS32 */
+#else /* WINDOWS32 */
+          /* On native Windows, access to a user's home directory
+             (via GetUserProfileDirectory) or to a user's environment
+             variables (via ExpandEnvironmentStringsForUser) requires
+             the credentials of the user.  Therefore we cannot support
+             the ~user syntax on this platform.
+             Handling ~user specially (and treat it like plain ~) if
+             user is getenv ("USERNAME") would not be a good idea,
+             since it would make people think that ~user is supported
+             in general.  */
+          if (flags & GLOB_TILDE_CHECK)
+            {
+              retval = GLOB_NOMATCH;
+              goto out;
+            }
+#endif /* WINDOWS32 */
         }
     }
 
@@ -1266,6 +1284,8 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 {
   size_t dirlen = strlen (directory);
   void *stream = NULL;
+  struct scratch_buffer s;
+  scratch_buffer_init (&s);
 # define GLOBNAMES_MEMBERS(nnames) \
     struct globnames *next; size_t count; char *name[nnames];
   struct globnames { GLOBNAMES_MEMBERS (FLEXIBLE_ARRAY_MEMBER) };
@@ -1337,6 +1357,7 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
         }
       else
         {
+          int dfd = dirfd (stream);
           int fnm_flags = ((!(flags & GLOB_PERIOD) ? FNM_PERIOD : 0)
                            | ((flags & GLOB_NOESCAPE) ? FNM_NOESCAPE : 0));
           flags |= GLOB_MAGCHAR;
@@ -1364,8 +1385,32 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
               if (flags & GLOB_ONLYDIR)
                 switch (readdir_result_type (d))
                   {
-                  case DT_DIR: case DT_LNK: case DT_UNKNOWN: break;
                   default: continue;
+                  case DT_DIR: break;
+                  case DT_LNK: case DT_UNKNOWN:
+                    /* The filesystem was too lazy to give us a hint,
+                       so we have to do it the hard way.  */
+                    if (__glibc_unlikely (dfd < 0 || flags & GLOB_ALTDIRFUNC))
+                      {
+                        size_t namelen = strlen (d.name);
+                        size_t need = dirlen + 1 + namelen + 1;
+                        if (s.length < need
+                            && !scratch_buffer_set_array_size (&s, need, 1))
+                          goto memory_error;
+                        char *p = mempcpy (s.data, directory, dirlen);
+                        *p = '/';
+                        p += p[-1] != '/';
+                        memcpy (p, d.name, namelen + 1);
+                        if (! is_dir (s.data, flags, pglob))
+                          continue;
+                      }
+                    else
+                      {
+                        struct_stat64 st64;
+                        if (! (GLOB_FSTATAT64 (dfd, d.name, &st64, 0) == 0
+                               && S_ISDIR (st64.st_mode)))
+                          continue;
+                      }
                   }
 
               if (fnmatch (pattern, d.name, fnm_flags) == 0)
@@ -1497,5 +1542,6 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
       __set_errno (save);
     }
 
+  scratch_buffer_free (&s);
   return result;
 }

Re: posix/glob.c: update from gnulib

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 1082 bytes --]

On 3/30/22 14:55, DJ Delorie via Libc-alpha wrote:

> Copied from gnulib/lib/glob.c in order to fix rhbz 1982608
> 
> Used config.h instead of libc-config.h

I don't see why this change is needed. This code is inside "#ifndef 
_LIBC" so this change should have no effect for glibc. And the change is 
harmful for Gnulib, since for this file Gnulib relies on including 
libc-config.h instead of plain config.h.

> The #ifdef around #define dirfd() was changed to #undef due to
> conflicts between glibc's internal and external definitions of
> dirfd().  This has been reported to gnulib.

I updated Gnulib to reflect this change; see first attached patch. That 
being said, I don't fully understand it. Wouldn't it be more efficient 
for glibc glob to use glibc's internal dirfd by whatever name you prefer?

Anyway, the only difference between what you proposed for glibc and 
current Gnulib glob is the second attached patch; could you please merge 
that into your proposal? That way, the two glob.c files can be 
identical, which is a good thing.

Thanks for following up on this.

[-- Attachment #2: glob-fixups.diff --]
[-- Type: text/x-patch, Size: 434 bytes --]

--- posix/glob.c	2022-03-30 16:18:03.358942602 -0700
+++ ../gnulib-savannah/lib/glob.c	2022-03-30 16:27:43.412646585 -0700
@@ -18,13 +18,13 @@
 #ifndef _LIBC
 
 /* Don't use __attribute__ __nonnull__ in this compilation unit.  Otherwise gcc
    optimizes away the pattern == NULL test below.  */
 # define _GL_ARG_NONNULL(params)
 
-# include <config.h>
+# include <libc-config.h>
 
 #endif
 
 #include <glob.h>
 
 #include <errno.h>

Re: posix/glob.c: update from gnulib

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 183 bytes --]

On 3/30/22 16:40, Paul Eggert wrote:
> 
> I updated Gnulib to reflect this change; see first attached patch.

Oops, forgot to attach that patch. Here it is. Also cc'ing to bug-gnulib.

[-- Attachment #2: 0001-glob-sync-better-with-glibc.patch --]
[-- Type: text/x-patch, Size: 1270 bytes --]

From 8fa9898afa5ee3da8f5d5a4797f98ae62b12d427 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 30 Mar 2022 16:29:11 -0700
Subject: [PATCH] glob: sync better with glibc

* lib/glob.c (dirfd) [_LIBC]: Use #undef instead of #ifdef.
Problem reported by DJ Delorie.
---
 ChangeLog  | 6 ++++++
 lib/glob.c | 5 ++---
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 7ea4f7797b..0f88fceed0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2022-03-30  Paul Eggert  <eggert@cs.ucla.edu>
+
+	glob: sync better with glibc
+	* lib/glob.c (dirfd) [_LIBC]: Use #undef instead of #ifdef.
+	Problem reported by DJ Delorie.
+
 2022-03-23  Paul Eggert  <eggert@cs.ucla.edu>
 
 	glob: test for glibc bug 25659
diff --git a/lib/glob.c b/lib/glob.c
index 52c79b4cd8..f6993a3706 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -57,9 +57,8 @@
 # define sysconf(id) __sysconf (id)
 # define closedir(dir) __closedir (dir)
 # define opendir(name) __opendir (name)
-# ifndef dirfd
-#  define dirfd(str) __dirfd (str)
-# endif
+# undef dirfd
+# define dirfd(str) __dirfd (str)
 # define readdir(str) __readdir64 (str)
 # define getpwnam_r(name, bufp, buf, len, res) \
     __getpwnam_r (name, bufp, buf, len, res)
-- 
2.35.1

Re: posix/glob.c: update from gnulib

[DJ Delorie]

Paul Eggert <eggert@cs.ucla.edu> writes:
>> Used config.h instead of libc-config.h
>
> I don't see why this change is needed. This code is inside "#ifndef 
> _LIBC" so this change should have no effect for glibc. And the change is 
> harmful for Gnulib, since for this file Gnulib relies on including 
> libc-config.h instead of plain config.h.

I was just leaving that line as it was in glibc, but as you note, it
shouldn't make a difference.  glibc does have a config.h though, and no
libc-config.h, which is what made me think this was significant.

>> The #ifdef around #define dirfd() was changed to #undef due to
>> conflicts between glibc's internal and external definitions of
>> dirfd().  This has been reported to gnulib.
>
> I updated Gnulib to reflect this change; see first attached patch. That 
> being said, I don't fully understand it. Wouldn't it be more efficient 
> for glibc glob to use glibc's internal dirfd by whatever name you prefer?

Perhaps, but doing so involved more than just using a macro, because a
lot of what the internal macro does depends on having insight into the
internals of other modules (like typedef names, for example), which
involves other includes (for those typedefs) and such.  It was far
easier to just leave it alone than to go down that (perhaps shallow ;)
rabbit hole.  I.e. it was the minimum fix.

Since it's just a dereference I don't think it will affect performance.

> Anyway, the only difference between what you proposed for glibc and 
> current Gnulib glob is the second attached patch; could you please merge 
> that into your proposal? That way, the two glob.c files can be 
> identical, which is a good thing.

Done.

Re: posix/glob.c: update from gnulib

[Adhemerval Zanella]

On 30/03/2022 18:55, DJ Delorie via Libc-alpha wrote:
> 
> Copied from gnulib/lib/glob.c in order to fix rhbz 1982608
> 
> Used config.h instead of libc-config.h
> 
> The #ifdef around #define dirfd() was changed to #undef due to
> conflicts between glibc's internal and external definitions of
> dirfd().  This has been reported to gnulib.


Could you create a bug report to track this? If I recall correctly from
gnulib discussion, this came from an issue with XFS. Would be possible
to craft a regression test?


> 
> diff --git a/posix/glob.c b/posix/glob.c
> index a2b5aabada..19b876f9f0 100644
> --- a/posix/glob.c
> +++ b/posix/glob.c
> @@ -28,6 +28,7 @@
>  #include <glob.h>
>  
>  #include <errno.h>
> +#include <fcntl.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
>  #include <stdbool.h>
> @@ -56,6 +57,8 @@
>  # define sysconf(id) __sysconf (id)
>  # define closedir(dir) __closedir (dir)
>  # define opendir(name) __opendir (name)
> +# undef dirfd
> +# define dirfd(str) __dirfd (str)
>  # define readdir(str) __readdir64 (str)
>  # define getpwnam_r(name, bufp, buf, len, res) \
>      __getpwnam_r (name, bufp, buf, len, res)
> @@ -69,11 +72,8 @@
>  # ifndef GLOB_LSTAT
>  #  define GLOB_LSTAT            gl_lstat
>  # endif
> -# ifndef GLOB_STAT64
> -#  define GLOB_STAT64           __stat64
> -# endif
> -# ifndef GLOB_LSTAT64
> -#  define GLOB_LSTAT64          __lstat64
> +# ifndef GLOB_FSTATAT64
> +#  define GLOB_FSTATAT64        __fstatat64
>  # endif
>  # include <shlib-compat.h>
>  #else /* !_LIBC */
> @@ -88,8 +88,7 @@
>  # define struct_stat            struct stat
>  # define struct_stat64          struct stat
>  # define GLOB_LSTAT             gl_lstat
> -# define GLOB_STAT64            stat
> -# define GLOB_LSTAT64           lstat
> +# define GLOB_FSTATAT64         fstatat
>  #endif /* _LIBC */
>  
>  #include <fnmatch.h>
> @@ -215,7 +214,8 @@ glob_lstat (glob_t *pglob, int flags, const char *fullname)
>    } ust;
>    return (__glibc_unlikely (flags & GLOB_ALTDIRFUNC)
>            ? pglob->GLOB_LSTAT (fullname, &ust.st)
> -          : GLOB_LSTAT64 (fullname, &ust.st64));
> +          : GLOB_FSTATAT64 (AT_FDCWD, fullname, &ust.st64,
> +                            AT_SYMLINK_NOFOLLOW));
>  }
>  
>  /* Set *R = A + B.  Return true if the answer is mathematically
> @@ -257,7 +257,8 @@ is_dir (char const *filename, int flags, glob_t const *pglob)
>    struct_stat64 st64;
>    return (__glibc_unlikely (flags & GLOB_ALTDIRFUNC)
>            ? pglob->gl_stat (filename, &st) == 0 && S_ISDIR (st.st_mode)
> -          : GLOB_STAT64 (filename, &st64) == 0 && S_ISDIR (st64.st_mode));
> +          : (GLOB_FSTATAT64 (AT_FDCWD, filename, &st64, 0) == 0
> +             && S_ISDIR (st64.st_mode)));
>  }
>  
>  /* Find the end of the sub-pattern in a brace expression.  */
> @@ -747,6 +748,8 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
>        else
>          {
>  #ifndef WINDOWS32
> +          /* Recognize ~user as a shorthand for the specified user's home
> +             directory.  */
>            char *end_name = strchr (dirname, '/');
>            char *user_name;
>            int malloc_user_name = 0;
> @@ -885,7 +888,22 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
>                }
>              scratch_buffer_free (&pwtmpbuf);
>            }
> -#endif /* !WINDOWS32 */
> +#else /* WINDOWS32 */
> +          /* On native Windows, access to a user's home directory
> +             (via GetUserProfileDirectory) or to a user's environment
> +             variables (via ExpandEnvironmentStringsForUser) requires
> +             the credentials of the user.  Therefore we cannot support
> +             the ~user syntax on this platform.
> +             Handling ~user specially (and treat it like plain ~) if
> +             user is getenv ("USERNAME") would not be a good idea,
> +             since it would make people think that ~user is supported
> +             in general.  */
> +          if (flags & GLOB_TILDE_CHECK)
> +            {
> +              retval = GLOB_NOMATCH;
> +              goto out;
> +            }
> +#endif /* WINDOWS32 */
>          }
>      }
>  
> @@ -1266,6 +1284,8 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
>  {
>    size_t dirlen = strlen (directory);
>    void *stream = NULL;
> +  struct scratch_buffer s;
> +  scratch_buffer_init (&s);
>  # define GLOBNAMES_MEMBERS(nnames) \
>      struct globnames *next; size_t count; char *name[nnames];
>    struct globnames { GLOBNAMES_MEMBERS (FLEXIBLE_ARRAY_MEMBER) };
> @@ -1337,6 +1357,7 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
>          }
>        else
>          {
> +          int dfd = dirfd (stream);
>            int fnm_flags = ((!(flags & GLOB_PERIOD) ? FNM_PERIOD : 0)
>                             | ((flags & GLOB_NOESCAPE) ? FNM_NOESCAPE : 0));
>            flags |= GLOB_MAGCHAR;
> @@ -1364,8 +1385,32 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
>                if (flags & GLOB_ONLYDIR)
>                  switch (readdir_result_type (d))
>                    {
> -                  case DT_DIR: case DT_LNK: case DT_UNKNOWN: break;
>                    default: continue;
> +                  case DT_DIR: break;
> +                  case DT_LNK: case DT_UNKNOWN:
> +                    /* The filesystem was too lazy to give us a hint,
> +                       so we have to do it the hard way.  */
> +                    if (__glibc_unlikely (dfd < 0 || flags & GLOB_ALTDIRFUNC))
> +                      {
> +                        size_t namelen = strlen (d.name);
> +                        size_t need = dirlen + 1 + namelen + 1;
> +                        if (s.length < need
> +                            && !scratch_buffer_set_array_size (&s, need, 1))
> +                          goto memory_error;
> +                        char *p = mempcpy (s.data, directory, dirlen);
> +                        *p = '/';
> +                        p += p[-1] != '/';
> +                        memcpy (p, d.name, namelen + 1);
> +                        if (! is_dir (s.data, flags, pglob))
> +                          continue;
> +                      }
> +                    else
> +                      {
> +                        struct_stat64 st64;
> +                        if (! (GLOB_FSTATAT64 (dfd, d.name, &st64, 0) == 0
> +                               && S_ISDIR (st64.st_mode)))
> +                          continue;
> +                      }
>                    }
>  
>                if (fnmatch (pattern, d.name, fnm_flags) == 0)
> @@ -1497,5 +1542,6 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
>        __set_errno (save);
>      }
>  
> +  scratch_buffer_free (&s);
>    return result;
>  }
> 

Re: posix/glob.c: update from gnulib

[DJ Delorie]

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> Could you create a bug report to track this?

It would be a copy of rhbz 1982608 and it originated in make, I don't
see any advantage of copying it just to sync with gnulib.


> If I recall correctly from gnulib discussion, this came from an issue
> with XFS. Would be possible to craft a regression test?

Not really.  It requires either an old XFS, or an XFS built with custom
command line options to emulate the old XFS.

Re: posix/glob.c: update from gnulib

[Adhemerval Zanella]

On 31/03/2022 14:38, DJ Delorie wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>> Could you create a bug report to track this?
> 
> It would be a copy of rhbz 1982608 and it originated in make, I don't
> see any advantage of copying it just to sync with gnulib.
> 

Yes, but we usually reference sourceware bug list for fixed bugs.  I think
mirroring the bug is better than just reference to an external database.

> 
>> If I recall correctly from gnulib discussion, this came from an issue
>> with XFS. Would be possible to craft a regression test?
> 
> Not really.  It requires either an old XFS, or an XFS built with custom
> command line options to emulate the old XFS.
> 

Re: posix/glob.c: update from gnulib

[DJ Delorie]

Conveniently...
https://sourceware.org/bugzilla/show_bug.cgi?id=25659

Re: posix/glob.c: update from gnulib

[Adhemerval Zanella]

On 31/03/2022 16:29, DJ Delorie wrote:
> 
> Conveniently...
> https://sourceware.org/bugzilla/show_bug.cgi?id=25659
> 

Thanks.

[v2] posix/glob.c: update from gnulib

[DJ Delorie]

Copied from gnulib/lib/glob.c in order to fix rhbz 1982608
Also fixes swbz 25659

diff --git a/posix/glob.c b/posix/glob.c
index a2b5aabada..f6993a3706 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -21,13 +21,14 @@
    optimizes away the pattern == NULL test below.  */
 # define _GL_ARG_NONNULL(params)
 
-# include <config.h>
+# include <libc-config.h>
 
 #endif
 
 #include <glob.h>
 
 #include <errno.h>
+#include <fcntl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <stdbool.h>
@@ -56,6 +57,8 @@
 # define sysconf(id) __sysconf (id)
 # define closedir(dir) __closedir (dir)
 # define opendir(name) __opendir (name)
+# undef dirfd
+# define dirfd(str) __dirfd (str)
 # define readdir(str) __readdir64 (str)
 # define getpwnam_r(name, bufp, buf, len, res) \
     __getpwnam_r (name, bufp, buf, len, res)
@@ -69,11 +72,8 @@
 # ifndef GLOB_LSTAT
 #  define GLOB_LSTAT            gl_lstat
 # endif
-# ifndef GLOB_STAT64
-#  define GLOB_STAT64           __stat64
-# endif
-# ifndef GLOB_LSTAT64
-#  define GLOB_LSTAT64          __lstat64
+# ifndef GLOB_FSTATAT64
+#  define GLOB_FSTATAT64        __fstatat64
 # endif
 # include <shlib-compat.h>
 #else /* !_LIBC */
@@ -88,8 +88,7 @@
 # define struct_stat            struct stat
 # define struct_stat64          struct stat
 # define GLOB_LSTAT             gl_lstat
-# define GLOB_STAT64            stat
-# define GLOB_LSTAT64           lstat
+# define GLOB_FSTATAT64         fstatat
 #endif /* _LIBC */
 
 #include <fnmatch.h>
@@ -215,7 +214,8 @@ glob_lstat (glob_t *pglob, int flags, const char *fullname)
   } ust;
   return (__glibc_unlikely (flags & GLOB_ALTDIRFUNC)
           ? pglob->GLOB_LSTAT (fullname, &ust.st)
-          : GLOB_LSTAT64 (fullname, &ust.st64));
+          : GLOB_FSTATAT64 (AT_FDCWD, fullname, &ust.st64,
+                            AT_SYMLINK_NOFOLLOW));
 }
 
 /* Set *R = A + B.  Return true if the answer is mathematically
@@ -257,7 +257,8 @@ is_dir (char const *filename, int flags, glob_t const *pglob)
   struct_stat64 st64;
   return (__glibc_unlikely (flags & GLOB_ALTDIRFUNC)
           ? pglob->gl_stat (filename, &st) == 0 && S_ISDIR (st.st_mode)
-          : GLOB_STAT64 (filename, &st64) == 0 && S_ISDIR (st64.st_mode));
+          : (GLOB_FSTATAT64 (AT_FDCWD, filename, &st64, 0) == 0
+             && S_ISDIR (st64.st_mode)));
 }
 
 /* Find the end of the sub-pattern in a brace expression.  */
@@ -747,6 +748,8 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
       else
         {
 #ifndef WINDOWS32
+          /* Recognize ~user as a shorthand for the specified user's home
+             directory.  */
           char *end_name = strchr (dirname, '/');
           char *user_name;
           int malloc_user_name = 0;
@@ -885,7 +888,22 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
               }
             scratch_buffer_free (&pwtmpbuf);
           }
-#endif /* !WINDOWS32 */
+#else /* WINDOWS32 */
+          /* On native Windows, access to a user's home directory
+             (via GetUserProfileDirectory) or to a user's environment
+             variables (via ExpandEnvironmentStringsForUser) requires
+             the credentials of the user.  Therefore we cannot support
+             the ~user syntax on this platform.
+             Handling ~user specially (and treat it like plain ~) if
+             user is getenv ("USERNAME") would not be a good idea,
+             since it would make people think that ~user is supported
+             in general.  */
+          if (flags & GLOB_TILDE_CHECK)
+            {
+              retval = GLOB_NOMATCH;
+              goto out;
+            }
+#endif /* WINDOWS32 */
         }
     }
 
@@ -1266,6 +1284,8 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 {
   size_t dirlen = strlen (directory);
   void *stream = NULL;
+  struct scratch_buffer s;
+  scratch_buffer_init (&s);
 # define GLOBNAMES_MEMBERS(nnames) \
     struct globnames *next; size_t count; char *name[nnames];
   struct globnames { GLOBNAMES_MEMBERS (FLEXIBLE_ARRAY_MEMBER) };
@@ -1337,6 +1357,7 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
         }
       else
         {
+          int dfd = dirfd (stream);
           int fnm_flags = ((!(flags & GLOB_PERIOD) ? FNM_PERIOD : 0)
                            | ((flags & GLOB_NOESCAPE) ? FNM_NOESCAPE : 0));
           flags |= GLOB_MAGCHAR;
@@ -1364,8 +1385,32 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
               if (flags & GLOB_ONLYDIR)
                 switch (readdir_result_type (d))
                   {
-                  case DT_DIR: case DT_LNK: case DT_UNKNOWN: break;
                   default: continue;
+                  case DT_DIR: break;
+                  case DT_LNK: case DT_UNKNOWN:
+                    /* The filesystem was too lazy to give us a hint,
+                       so we have to do it the hard way.  */
+                    if (__glibc_unlikely (dfd < 0 || flags & GLOB_ALTDIRFUNC))
+                      {
+                        size_t namelen = strlen (d.name);
+                        size_t need = dirlen + 1 + namelen + 1;
+                        if (s.length < need
+                            && !scratch_buffer_set_array_size (&s, need, 1))
+                          goto memory_error;
+                        char *p = mempcpy (s.data, directory, dirlen);
+                        *p = '/';
+                        p += p[-1] != '/';
+                        memcpy (p, d.name, namelen + 1);
+                        if (! is_dir (s.data, flags, pglob))
+                          continue;
+                      }
+                    else
+                      {
+                        struct_stat64 st64;
+                        if (! (GLOB_FSTATAT64 (dfd, d.name, &st64, 0) == 0
+                               && S_ISDIR (st64.st_mode)))
+                          continue;
+                      }
                   }
 
               if (fnmatch (pattern, d.name, fnm_flags) == 0)
@@ -1497,5 +1542,6 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
       __set_errno (save);
     }
 
+  scratch_buffer_free (&s);
   return result;
 }

Re: [v2] posix/glob.c: update from gnulib

[Paul Eggert]

Thanks, this looks good to me.

Re: [v2] posix/glob.c: update from gnulib

[Carlos O'Donell]

On 3/31/22 21:55, Paul Eggert wrote:
> Thanks, this looks good to me.
> 

Fails CI/CD on i686 :>

https://patchwork.sourceware.org/project/glibc/patch/xnmth5vqnv.fsf@greed.delorie.com/

In file included from ../sysdeps/unix/sysv/linux/glob64-time64.c:43:
../posix/glob.c: In function 'glob_lstat':
../posix/glob.c:217:49: error: passing argument 3 of '__fstatat64' from incompatible pointer type [-Werror=incompatible-pointer-types]
  217 |           : GLOB_FSTATAT64 (AT_FDCWD, fullname, &ust.st64,
      |                                                 ^~~~~~~~~
      |                                                 |
      |                                                 struct __stat64_t64 *

-- 
Cheers,
Carlos.

[v3] posix/glob.c: update from gnulib

[DJ Delorie]

[v3: Added sysdeps/unix/sysv/linux/glob64-time64.c to fix i686 build]

Copied from gnulib/lib/glob.c in order to fix rhbz 1982608
Also fixes swbz 25659

diff --git a/posix/glob.c b/posix/glob.c
index a2b5aabada..f6993a3706 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -21,13 +21,14 @@
    optimizes away the pattern == NULL test below.  */
 # define _GL_ARG_NONNULL(params)
 
-# include <config.h>
+# include <libc-config.h>
 
 #endif
 
 #include <glob.h>
 
 #include <errno.h>
+#include <fcntl.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <stdbool.h>
@@ -56,6 +57,8 @@
 # define sysconf(id) __sysconf (id)
 # define closedir(dir) __closedir (dir)
 # define opendir(name) __opendir (name)
+# undef dirfd
+# define dirfd(str) __dirfd (str)
 # define readdir(str) __readdir64 (str)
 # define getpwnam_r(name, bufp, buf, len, res) \
     __getpwnam_r (name, bufp, buf, len, res)
@@ -69,11 +72,8 @@
 # ifndef GLOB_LSTAT
 #  define GLOB_LSTAT            gl_lstat
 # endif
-# ifndef GLOB_STAT64
-#  define GLOB_STAT64           __stat64
-# endif
-# ifndef GLOB_LSTAT64
-#  define GLOB_LSTAT64          __lstat64
+# ifndef GLOB_FSTATAT64
+#  define GLOB_FSTATAT64        __fstatat64
 # endif
 # include <shlib-compat.h>
 #else /* !_LIBC */
@@ -88,8 +88,7 @@
 # define struct_stat            struct stat
 # define struct_stat64          struct stat
 # define GLOB_LSTAT             gl_lstat
-# define GLOB_STAT64            stat
-# define GLOB_LSTAT64           lstat
+# define GLOB_FSTATAT64         fstatat
 #endif /* _LIBC */
 
 #include <fnmatch.h>
@@ -215,7 +214,8 @@ glob_lstat (glob_t *pglob, int flags, const char *fullname)
   } ust;
   return (__glibc_unlikely (flags & GLOB_ALTDIRFUNC)
           ? pglob->GLOB_LSTAT (fullname, &ust.st)
-          : GLOB_LSTAT64 (fullname, &ust.st64));
+          : GLOB_FSTATAT64 (AT_FDCWD, fullname, &ust.st64,
+                            AT_SYMLINK_NOFOLLOW));
 }
 
 /* Set *R = A + B.  Return true if the answer is mathematically
@@ -257,7 +257,8 @@ is_dir (char const *filename, int flags, glob_t const *pglob)
   struct_stat64 st64;
   return (__glibc_unlikely (flags & GLOB_ALTDIRFUNC)
           ? pglob->gl_stat (filename, &st) == 0 && S_ISDIR (st.st_mode)
-          : GLOB_STAT64 (filename, &st64) == 0 && S_ISDIR (st64.st_mode));
+          : (GLOB_FSTATAT64 (AT_FDCWD, filename, &st64, 0) == 0
+             && S_ISDIR (st64.st_mode)));
 }
 
 /* Find the end of the sub-pattern in a brace expression.  */
@@ -747,6 +748,8 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
       else
         {
 #ifndef WINDOWS32
+          /* Recognize ~user as a shorthand for the specified user's home
+             directory.  */
           char *end_name = strchr (dirname, '/');
           char *user_name;
           int malloc_user_name = 0;
@@ -885,7 +888,22 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
               }
             scratch_buffer_free (&pwtmpbuf);
           }
-#endif /* !WINDOWS32 */
+#else /* WINDOWS32 */
+          /* On native Windows, access to a user's home directory
+             (via GetUserProfileDirectory) or to a user's environment
+             variables (via ExpandEnvironmentStringsForUser) requires
+             the credentials of the user.  Therefore we cannot support
+             the ~user syntax on this platform.
+             Handling ~user specially (and treat it like plain ~) if
+             user is getenv ("USERNAME") would not be a good idea,
+             since it would make people think that ~user is supported
+             in general.  */
+          if (flags & GLOB_TILDE_CHECK)
+            {
+              retval = GLOB_NOMATCH;
+              goto out;
+            }
+#endif /* WINDOWS32 */
         }
     }
 
@@ -1266,6 +1284,8 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 {
   size_t dirlen = strlen (directory);
   void *stream = NULL;
+  struct scratch_buffer s;
+  scratch_buffer_init (&s);
 # define GLOBNAMES_MEMBERS(nnames) \
     struct globnames *next; size_t count; char *name[nnames];
   struct globnames { GLOBNAMES_MEMBERS (FLEXIBLE_ARRAY_MEMBER) };
@@ -1337,6 +1357,7 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
         }
       else
         {
+          int dfd = dirfd (stream);
           int fnm_flags = ((!(flags & GLOB_PERIOD) ? FNM_PERIOD : 0)
                            | ((flags & GLOB_NOESCAPE) ? FNM_NOESCAPE : 0));
           flags |= GLOB_MAGCHAR;
@@ -1364,8 +1385,32 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
               if (flags & GLOB_ONLYDIR)
                 switch (readdir_result_type (d))
                   {
-                  case DT_DIR: case DT_LNK: case DT_UNKNOWN: break;
                   default: continue;
+                  case DT_DIR: break;
+                  case DT_LNK: case DT_UNKNOWN:
+                    /* The filesystem was too lazy to give us a hint,
+                       so we have to do it the hard way.  */
+                    if (__glibc_unlikely (dfd < 0 || flags & GLOB_ALTDIRFUNC))
+                      {
+                        size_t namelen = strlen (d.name);
+                        size_t need = dirlen + 1 + namelen + 1;
+                        if (s.length < need
+                            && !scratch_buffer_set_array_size (&s, need, 1))
+                          goto memory_error;
+                        char *p = mempcpy (s.data, directory, dirlen);
+                        *p = '/';
+                        p += p[-1] != '/';
+                        memcpy (p, d.name, namelen + 1);
+                        if (! is_dir (s.data, flags, pglob))
+                          continue;
+                      }
+                    else
+                      {
+                        struct_stat64 st64;
+                        if (! (GLOB_FSTATAT64 (dfd, d.name, &st64, 0) == 0
+                               && S_ISDIR (st64.st_mode)))
+                          continue;
+                      }
                   }
 
               if (fnmatch (pattern, d.name, fnm_flags) == 0)
@@ -1497,5 +1542,6 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
       __set_errno (save);
     }
 
+  scratch_buffer_free (&s);
   return result;
 }
diff --git a/sysdeps/unix/sysv/linux/glob64-time64.c b/sysdeps/unix/sysv/linux/glob64-time64.c
index 32b4929a1b..1d485dc806 100644
--- a/sysdeps/unix/sysv/linux/glob64-time64.c
+++ b/sysdeps/unix/sysv/linux/glob64-time64.c
@@ -37,6 +37,7 @@
 # define GLOB_LSTAT      gl_lstat
 # define GLOB_STAT64     __stat64_time64
 # define GLOB_LSTAT64    __lstat64_time64
+# define GLOB_FSTATAT64    __fstatat64_time64
 
 # define COMPILE_GLOB64	1
 

Re: [v3] posix/glob.c: update from gnulib

[Carlos O'Donell]

On 4/14/22 19:40, DJ Delorie wrote:
> 
> [v3: Added sysdeps/unix/sysv/linux/glob64-time64.c to fix i686 build]
> 
> Copied from gnulib/lib/glob.c in order to fix rhbz 1982608
> Also fixes swbz 25659

Passes CI/CD. Passes without regression on x86_64. Minor difference from upstream gnulib.

LGTM. Thank you!

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Tested-by: Carlos O'Donell <carlos@redhat.com>

> diff --git a/posix/glob.c b/posix/glob.c
> index a2b5aabada..f6993a3706 100644
> --- a/posix/glob.c
> +++ b/posix/glob.c
> @@ -21,13 +21,14 @@
>     optimizes away the pattern == NULL test below.  */
>  # define _GL_ARG_NONNULL(params)
>  
> -# include <config.h>
> +# include <libc-config.h>
>  
>  #endif
>  
>  #include <glob.h>
>  
>  #include <errno.h>
> +#include <fcntl.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
>  #include <stdbool.h>
> @@ -56,6 +57,8 @@
>  # define sysconf(id) __sysconf (id)
>  # define closedir(dir) __closedir (dir)
>  # define opendir(name) __opendir (name)
> +# undef dirfd
> +# define dirfd(str) __dirfd (str)
>  # define readdir(str) __readdir64 (str)
>  # define getpwnam_r(name, bufp, buf, len, res) \
>      __getpwnam_r (name, bufp, buf, len, res)
> @@ -69,11 +72,8 @@
>  # ifndef GLOB_LSTAT
>  #  define GLOB_LSTAT            gl_lstat
>  # endif
> -# ifndef GLOB_STAT64
> -#  define GLOB_STAT64           __stat64
> -# endif
> -# ifndef GLOB_LSTAT64
> -#  define GLOB_LSTAT64          __lstat64
> +# ifndef GLOB_FSTATAT64
> +#  define GLOB_FSTATAT64        __fstatat64
>  # endif
>  # include <shlib-compat.h>
>  #else /* !_LIBC */
> @@ -88,8 +88,7 @@
>  # define struct_stat            struct stat
>  # define struct_stat64          struct stat
>  # define GLOB_LSTAT             gl_lstat
> -# define GLOB_STAT64            stat
> -# define GLOB_LSTAT64           lstat
> +# define GLOB_FSTATAT64         fstatat
>  #endif /* _LIBC */
>  
>  #include <fnmatch.h>
> @@ -215,7 +214,8 @@ glob_lstat (glob_t *pglob, int flags, const char *fullname)
>    } ust;
>    return (__glibc_unlikely (flags & GLOB_ALTDIRFUNC)
>            ? pglob->GLOB_LSTAT (fullname, &ust.st)
> -          : GLOB_LSTAT64 (fullname, &ust.st64));
> +          : GLOB_FSTATAT64 (AT_FDCWD, fullname, &ust.st64,
> +                            AT_SYMLINK_NOFOLLOW));
>  }
>  
>  /* Set *R = A + B.  Return true if the answer is mathematically
> @@ -257,7 +257,8 @@ is_dir (char const *filename, int flags, glob_t const *pglob)
>    struct_stat64 st64;
>    return (__glibc_unlikely (flags & GLOB_ALTDIRFUNC)
>            ? pglob->gl_stat (filename, &st) == 0 && S_ISDIR (st.st_mode)
> -          : GLOB_STAT64 (filename, &st64) == 0 && S_ISDIR (st64.st_mode));
> +          : (GLOB_FSTATAT64 (AT_FDCWD, filename, &st64, 0) == 0
> +             && S_ISDIR (st64.st_mode)));
>  }
>  
>  /* Find the end of the sub-pattern in a brace expression.  */
> @@ -747,6 +748,8 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
>        else
>          {
>  #ifndef WINDOWS32
> +          /* Recognize ~user as a shorthand for the specified user's home
> +             directory.  */
>            char *end_name = strchr (dirname, '/');
>            char *user_name;
>            int malloc_user_name = 0;
> @@ -885,7 +888,22 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
>                }
>              scratch_buffer_free (&pwtmpbuf);
>            }
> -#endif /* !WINDOWS32 */
> +#else /* WINDOWS32 */
> +          /* On native Windows, access to a user's home directory
> +             (via GetUserProfileDirectory) or to a user's environment
> +             variables (via ExpandEnvironmentStringsForUser) requires
> +             the credentials of the user.  Therefore we cannot support
> +             the ~user syntax on this platform.
> +             Handling ~user specially (and treat it like plain ~) if
> +             user is getenv ("USERNAME") would not be a good idea,
> +             since it would make people think that ~user is supported
> +             in general.  */
> +          if (flags & GLOB_TILDE_CHECK)
> +            {
> +              retval = GLOB_NOMATCH;
> +              goto out;
> +            }
> +#endif /* WINDOWS32 */
>          }
>      }
>  
> @@ -1266,6 +1284,8 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
>  {
>    size_t dirlen = strlen (directory);
>    void *stream = NULL;
> +  struct scratch_buffer s;
> +  scratch_buffer_init (&s);
>  # define GLOBNAMES_MEMBERS(nnames) \
>      struct globnames *next; size_t count; char *name[nnames];
>    struct globnames { GLOBNAMES_MEMBERS (FLEXIBLE_ARRAY_MEMBER) };
> @@ -1337,6 +1357,7 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
>          }
>        else
>          {
> +          int dfd = dirfd (stream);
>            int fnm_flags = ((!(flags & GLOB_PERIOD) ? FNM_PERIOD : 0)
>                             | ((flags & GLOB_NOESCAPE) ? FNM_NOESCAPE : 0));
>            flags |= GLOB_MAGCHAR;
> @@ -1364,8 +1385,32 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
>                if (flags & GLOB_ONLYDIR)
>                  switch (readdir_result_type (d))
>                    {
> -                  case DT_DIR: case DT_LNK: case DT_UNKNOWN: break;
>                    default: continue;
> +                  case DT_DIR: break;
> +                  case DT_LNK: case DT_UNKNOWN:
> +                    /* The filesystem was too lazy to give us a hint,
> +                       so we have to do it the hard way.  */
> +                    if (__glibc_unlikely (dfd < 0 || flags & GLOB_ALTDIRFUNC))
> +                      {
> +                        size_t namelen = strlen (d.name);
> +                        size_t need = dirlen + 1 + namelen + 1;
> +                        if (s.length < need
> +                            && !scratch_buffer_set_array_size (&s, need, 1))
> +                          goto memory_error;
> +                        char *p = mempcpy (s.data, directory, dirlen);
> +                        *p = '/';
> +                        p += p[-1] != '/';
> +                        memcpy (p, d.name, namelen + 1);
> +                        if (! is_dir (s.data, flags, pglob))
> +                          continue;
> +                      }
> +                    else
> +                      {
> +                        struct_stat64 st64;
> +                        if (! (GLOB_FSTATAT64 (dfd, d.name, &st64, 0) == 0
> +                               && S_ISDIR (st64.st_mode)))
> +                          continue;
> +                      }
>                    }
>  
>                if (fnmatch (pattern, d.name, fnm_flags) == 0)
> @@ -1497,5 +1542,6 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
>        __set_errno (save);
>      }
>  
> +  scratch_buffer_free (&s);
>    return result;
>  }
> diff --git a/sysdeps/unix/sysv/linux/glob64-time64.c b/sysdeps/unix/sysv/linux/glob64-time64.c
> index 32b4929a1b..1d485dc806 100644
> --- a/sysdeps/unix/sysv/linux/glob64-time64.c
> +++ b/sysdeps/unix/sysv/linux/glob64-time64.c
> @@ -37,6 +37,7 @@
>  # define GLOB_LSTAT      gl_lstat
>  # define GLOB_STAT64     __stat64_time64
>  # define GLOB_LSTAT64    __lstat64_time64
> +# define GLOB_FSTATAT64    __fstatat64_time64
>  
>  # define COMPILE_GLOB64	1
>  
> 


-- 
Cheers,
Carlos.

Re: [v3] posix/glob.c: update from gnulib

[DJ Delorie]

Thanks!  Pushed - and with attribution this time!