[patch] glob: resolve DT_UNKNOWN via is_dir

[patch] glob: resolve DT_UNKNOWN via is_dir

[DJ Delorie]

The DT_* values returned by getdents (readdir) are only hints and
not required.  In fact, some Linux filesystems return DT_UNKNOWN
for most entries, regardless of actual type.  This causes make
to mis-match patterns with a trailing slash (via GLOB_ONLYDIR)
(see make's functions/wildcard test case).  Thus, this patch
detects that case and uses is_dir() to make the type known enough
for proper operation.

Performance in non-DT_UNKNOWN cases is not affected.

The lack of DT_* is a well known issue on older XFS installations
(for example, RHEL 7 and 8, Fedora 28) but can be recreated by
creating an XFS filesystem with flags that mimic older behavior:

$ fallocate -l 10G /xfs.fs
$ mkfs.xfs -n ftype=0 -m crc=0 -f /xfs.fs
$ mkdir /xfs
$ mount -o loop /xfs.fs /xfs

[tested by importing this change to glibc, and using that to run make's testsuite]

diff --git a/lib/glob.c b/lib/glob.c
index 22c459574..d0521bb4a 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -1381,7 +1381,26 @@ 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;
+                  case DT_DIR: case DT_LNK: break;
+		  case DT_UNKNOWN:
+		    {
+		      /* The filesystem was too lazy to give us a hint,
+			 so we have to do it the hard way.  */
+		      char *fullpath, *p;
+		      bool isdir;
+		      fullpath = malloc (strlen (directory) + strlen (d.name) + 2);
+		      if (fullpath == NULL)
+			/* This matches old behavior wrt DT_UNKNOWN.  */
+			break;
+		      p = stpcpy (fullpath, directory);
+		      *p++ = '/';
+		      strcpy (p, d.name);
+		      isdir = is_dir (fullpath);
+		      free (fullpath);
+		      if (isdir)
+			break;
+		      continue;
+		    }
                   default: continue;
                   }
 

Re: [patch] glob: resolve DT_UNKNOWN via is_dir

[Florian Weimer]

* DJ Delorie:

> diff --git a/lib/glob.c b/lib/glob.c
> index 22c459574..d0521bb4a 100644
> --- a/lib/glob.c
> +++ b/lib/glob.c
> @@ -1381,7 +1381,26 @@ 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;
> +                  case DT_DIR: case DT_LNK: break;
> +		  case DT_UNKNOWN:
> +		    {
> +		      /* The filesystem was too lazy to give us a hint,
> +			 so we have to do it the hard way.  */
> +		      char *fullpath, *p;
> +		      bool isdir;
> +		      fullpath = malloc (strlen (directory) + strlen (d.name) + 2);
> +		      if (fullpath == NULL)
> +			/* This matches old behavior wrt DT_UNKNOWN.  */
> +			break;

Shouldn't this report memory allocation failure to the caller?

Thanks,
Florian

Re: [patch] glob: resolve DT_UNKNOWN via is_dir

[DJ Delorie]

Florian Weimer <fweimer@redhat.com> writes:
>> +		      if (fullpath == NULL)
>> +			/* This matches old behavior wrt DT_UNKNOWN.  */
>> +			break;
>
> Shouldn't this report memory allocation failure to the caller?

We could easily replace that break with a "goto memory_error" but that
changes the new logic from "better hint" to "mandatory function".

Re: [patch] glob: resolve DT_UNKNOWN via is_dir

[Florian Weimer]

* DJ Delorie:

> Florian Weimer <fweimer@redhat.com> writes:
>>> +		      if (fullpath == NULL)
>>> +			/* This matches old behavior wrt DT_UNKNOWN.  */
>>> +			break;
>>
>> Shouldn't this report memory allocation failure to the caller?
>
> We could easily replace that break with a "goto memory_error" but that
> changes the new logic from "better hint" to "mandatory function".

I thought the bug is that it's not actually a hint?  That DT_UNKNOWN
here leads to incorrect results in glob?

Thanks,
Florian

Re: [patch] glob: resolve DT_UNKNOWN via is_dir

[DJ Delorie]

Florian Weimer <fweimer@redhat.com> writes:
> I thought the bug is that it's not actually a hint?  That DT_UNKNOWN
> here leads to incorrect results in glob?

All the DT_* are hints, according to the man pages, which say that
DT_UNKNOWN must be handled by the caller.  So I guess it depends on
whether the original developers knew this and weren't worried about
accuracy, or if there were no filesystems that ever returned DT_UNKNOWN
for regular files, so it was decided to always include "I don't know"'s
as a "future proofing" option.

Doesn't matter to me either way, of course.

[patch v2] glob: resolve DT_UNKNOWN via is_dir

[DJ Delorie]

[v2: changed malloc failure from ignore to error; added support for
alloca; tested by copying to glibc and testing there]

The DT_* values returned by getdents (readdir) are only hints and
not required.  In fact, some Linux filesystems return DT_UNKNOWN
for most entries, regardless of actual type.  This causes make
to mis-match patterns with a trailing slash (via GLOB_ONLYDIR)
(see make's functions/wildcard test case).  Thus, this patch
detects that case and uses is_dir() to make the type known enough
for proper operation.

Performance in non-DT_UNKNOWN cases is not affected.

The lack of DT_* is a well known issue on older XFS installations
(for example, RHEL 7 and 8, Fedora 28) but can be recreated by
creating an XFS filesystem with flags that mimic older behavior:

$ fallocate -l 10G /xfs.fs
$ mkfs.xfs -n ftype=0 -m crc=0 -f /xfs.fs
$ mkdir /xfs
$ mount -o loop /xfs.fs /xfs

diff --git a/lib/glob.c b/lib/glob.c
index f8d8a306f2..c28c92a42c 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -1381,7 +1381,33 @@ 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;
+                  case DT_DIR: case DT_LNK: break;
+		  case DT_UNKNOWN:
+		    {
+		      /* The filesystem was too lazy to give us a hint,
+			 so we have to do it the hard way.  */
+		      char *fullpath, *p;
+		      bool isdir;
+		      int need = strlen (directory) + strlen (d.name) + 2;
+		      int use_alloca = glob_use_alloca (alloca_used, need);
+		      if (use_alloca)
+			fullpath = alloca_account (need, alloca_used);
+		      else
+			{
+			  fullpath = malloc (need);
+			  if (fullpath == NULL)
+			    goto memory_error;
+			}
+		      p = stpcpy (fullpath, directory);
+		      *p++ = '/';
+		      strcpy (p, d.name);
+		      isdir = is_dir (fullpath, flags, pglob);
+		      if (!use_alloca)
+			free (fullpath);
+		      if (isdir)
+			break;
+		      continue;
+		    }
                   default: continue;
                   }
 

Re: [patch v2] glob: resolve DT_UNKNOWN via is_dir

[Paul Eggert]

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

Thanks for looking into this; it's long been on my plate but I haven't 
had time to work on the proper solution, which is basically to rewrite 
glob from scratch (this should make it considerably faster).

As far as your patch goes:

Gnulib prefers spaces to tabs.

The code unnecessarily calls strlen (directory).

The patch mishandles a directory "/" and a file "x", as it stats "//x" 
but this may differ from "/x" on some systems.

For speed the code should prefer fstatat on d.name to stat on the full 
name. Too bad glob_t doesn't have a gl_fstatat entry, but we can use 
fstatat when GLOB_ALTDIRFUNC is not in use.

The patch goes through a loop calling alloca_account, which is not good: 
it'll waste the stack. Better to use a scratch buffer, as in other parts 
of glob.c.

The code should prefer mempcpy and memcpy to stpcpy and strcpy (as in 
the rest of glob.c). That way, the Gnulib glob module needn't depend on 
the stpcpy module (plus the code's more reliable if another thread 
stomps on our strings between strlen and strcpy :-).

I don't see how the patch fixes the case where readdir_result_type (d) 
returns DT_LNK; this might be a symlink to a directory.

Proposed pair of patches attached (I haven't installed these). The first 
is yours but with tabs turned to spaces and with ChangeLog equal to your 
log entry. The second contains my proposed improvements. I haven't 
tested except on the Gnulib test cases (which aren't much).

Of course performance will suffer with all these correctness patches, 
but that can wait until a rewrite.

[-- Attachment #2: 0001-glob-resolve-DT_UNKNOWN-via-is_dir.patch --]
[-- Type: text/x-patch, Size: 3998 bytes --]

From 69247bc996c50da0564f6157358ba99b13abbd16 Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Fri, 11 Mar 2022 16:43:39 -0500
Subject: [PATCH 1/2] glob: resolve DT_UNKNOWN via is_dir

[v2: changed malloc failure from ignore to error; added support for
alloca; tested by copying to glibc and testing there]

The DT_* values returned by getdents (readdir) are only hints and
not required.  In fact, some Linux filesystems return DT_UNKNOWN
for most entries, regardless of actual type.  This causes make
to mis-match patterns with a trailing slash (via GLOB_ONLYDIR)
(see make's functions/wildcard test case).  Thus, this patch
detects that case and uses is_dir() to make the type known enough
for proper operation.

Performance in non-DT_UNKNOWN cases is not affected.

The lack of DT_* is a well known issue on older XFS installations
(for example, RHEL 7 and 8, Fedora 28) but can be recreated by
creating an XFS filesystem with flags that mimic older behavior:

$ fallocate -l 10G /xfs.fs
$ mkfs.xfs -n ftype=0 -m crc=0 -f /xfs.fs
$ mkdir /xfs
$ mount -o loop /xfs.fs /xfs
---
 ChangeLog  | 23 +++++++++++++++++++++++
 lib/glob.c | 28 +++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 7a6ade78c3..f39c749ad3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,26 @@
+2022-03-11  DJ Delorie  <dj@redhat.com>
+
+	glob: resolve DT_UNKNOWN via is_dir
+
+	The DT_* values returned by getdents (readdir) are only hints and
+	not required.  In fact, some Linux filesystems return DT_UNKNOWN
+	for most entries, regardless of actual type.  This causes make
+	to mis-match patterns with a trailing slash (via GLOB_ONLYDIR)
+	(see make's functions/wildcard test case).  Thus, this patch
+	detects that case and uses is_dir() to make the type known enough
+	for proper operation.
+
+	Performance in non-DT_UNKNOWN cases is not affected.
+
+	The lack of DT_* is a well known issue on older XFS installations
+	(for example, RHEL 7 and 8, Fedora 28) but can be recreated by
+	creating an XFS filesystem with flags that mimic older behavior:
+
+	$ fallocate -l 10G /xfs.fs
+	$ mkfs.xfs -n ftype=0 -m crc=0 -f /xfs.fs
+	$ mkdir /xfs
+	$ mount -o loop /xfs.fs /xfs
+
 2022-03-11  Paul Eggert  <eggert@cs.ucla.edu>
 
 	regex: fix minor over-allocation
diff --git a/lib/glob.c b/lib/glob.c
index f8d8a306f2..0da46ac138 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -1381,7 +1381,33 @@ 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;
+                  case DT_DIR: case DT_LNK: break;
+                  case DT_UNKNOWN:
+                    {
+                      /* The filesystem was too lazy to give us a hint,
+                         so we have to do it the hard way.  */
+                      char *fullpath, *p;
+                      bool isdir;
+                      int need = strlen (directory) + strlen (d.name) + 2;
+                      int use_alloca = glob_use_alloca (alloca_used, need);
+                      if (use_alloca)
+                        fullpath = alloca_account (need, alloca_used);
+                      else
+                        {
+                          fullpath = malloc (need);
+                          if (fullpath == NULL)
+                            goto memory_error;
+                        }
+                      p = stpcpy (fullpath, directory);
+                      *p++ = '/';
+                      strcpy (p, d.name);
+                      isdir = is_dir (fullpath, flags, pglob);
+                      if (!use_alloca)
+                        free (fullpath);
+                      if (isdir)
+                        break;
+                      continue;
+                    }
                   default: continue;
                   }
 
-- 
2.35.1


[-- Attachment #3: 0002-glob-fix-symlink-and-issues-improve-speed.patch --]
[-- Type: text/x-patch, Size: 9047 bytes --]

From 1682446c7bd0bf41c61816258096c6f6515d27f6 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Fri, 11 Mar 2022 16:35:59 -0800
Subject: [PATCH 2/2] glob: fix symlink and // issues; improve speed

* lib/glob.c: Include fcntl.h.
(dirfd) [_LIBC]: New macro.
(GLOB_STAT64, GLOB_LSTAT64): Remove.  Replace all uses with ...
(GLOB_FSTATAT64): ... this new macro.
(glob_in_dir): Treat DT_LNK like DT_UNKNOWN.
Use directory-relative fstatat unless GLOB_ALTDIRFUNC, or dirfd fails.
Avoid duplicate strlen (directory).
Work even if directory is "/", without turning it into "//".
Use a scratch buffer instead of by-hand alloca stuff.
Use mempcpy and memcpy instead of stpcpy and strcpy.
* modules/glob (Depends-on): Add dirfd, fstatat; remove stat.
---
 ChangeLog    | 15 +++++++++++
 lib/glob.c   | 74 +++++++++++++++++++++++++++-------------------------
 modules/glob |  3 ++-
 3 files changed, 55 insertions(+), 37 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index f39c749ad3..88bb4db70a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2022-03-11  Paul Eggert  <eggert@cs.ucla.edu>
+
+	glob: fix symlink and // issues; improve speed
+	* lib/glob.c: Include fcntl.h.
+	(dirfd) [_LIBC]: New macro.
+	(GLOB_STAT64, GLOB_LSTAT64): Remove.  Replace all uses with ...
+	(GLOB_FSTATAT64): ... this new macro.
+	(glob_in_dir): Treat DT_LNK like DT_UNKNOWN.
+	Use directory-relative fstatat unless GLOB_ALTDIRFUNC, or dirfd fails.
+	Avoid duplicate strlen (directory).
+	Work even if directory is "/", without turning it into "//".
+	Use a scratch buffer instead of by-hand alloca stuff.
+	Use mempcpy and memcpy instead of stpcpy and strcpy.
+	* modules/glob (Depends-on): Add dirfd, fstatat; remove stat.
+
 2022-03-11  DJ Delorie  <dj@redhat.com>
 
 	glob: resolve DT_UNKNOWN via is_dir
diff --git a/lib/glob.c b/lib/glob.c
index 0da46ac138..5afb178153 100644
--- a/lib/glob.c
+++ b/lib/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,7 @@
 # define sysconf(id) __sysconf (id)
 # define closedir(dir) __closedir (dir)
 # define opendir(name) __opendir (name)
+# 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 +71,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 +87,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 +213,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 +256,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.  */
@@ -1283,6 +1283,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) };
@@ -1354,6 +1356,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;
@@ -1381,34 +1384,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: break;
-                  case DT_UNKNOWN:
-                    {
-                      /* The filesystem was too lazy to give us a hint,
-                         so we have to do it the hard way.  */
-                      char *fullpath, *p;
-                      bool isdir;
-                      int need = strlen (directory) + strlen (d.name) + 2;
-                      int use_alloca = glob_use_alloca (alloca_used, need);
-                      if (use_alloca)
-                        fullpath = alloca_account (need, alloca_used);
-                      else
-                        {
-                          fullpath = malloc (need);
-                          if (fullpath == NULL)
-                            goto memory_error;
-                        }
-                      p = stpcpy (fullpath, directory);
-                      *p++ = '/';
-                      strcpy (p, d.name);
-                      isdir = is_dir (fullpath, flags, pglob);
-                      if (!use_alloca)
-                        free (fullpath);
-                      if (isdir)
-                        break;
-                      continue;
-                    }
                   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)
@@ -1540,5 +1541,6 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
       __set_errno (save);
     }
 
+  scratch_buffer_free (&s);
   return result;
 }
diff --git a/modules/glob b/modules/glob
index ba270e8427..a43b71cfbb 100644
--- a/modules/glob
+++ b/modules/glob
@@ -17,8 +17,10 @@ alloca          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 builtin-expect  [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 closedir        [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 d-type          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+dirfd           [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 flexmember      [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 fnmatch         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+fstatat         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 getlogin_r      [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 libc-config     [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 memchr          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
@@ -26,7 +28,6 @@ mempcpy         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 opendir         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 readdir         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 scratch_buffer  [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-stat            [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 stdbool         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 stdint          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 strdup          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-- 
2.35.1

Re: [patch v2] glob: resolve DT_UNKNOWN via is_dir

[DJ Delorie]

Paul Eggert <eggert@cs.ucla.edu> writes:
> I don't see how the patch fixes the case where readdir_result_type (d) 
> returns DT_LNK; this might be a symlink to a directory.

It was not part of the problem I was trying to solve.  A DT_LNK was
already a known file type, and handled, so I left it alone.

> Proposed pair of patches attached (I haven't installed these). The first 
> is yours but with tabs turned to spaces and with ChangeLog equal to your 
> log entry. The second contains my proposed improvements. I haven't 
> tested except on the Gnulib test cases (which aren't much).

I tested it on upstream glibc; it needs one #undef but was otherwise ok.

Reviewed-by: DJ Delorie <dj@redhat.com>

> Of course performance will suffer with all these correctness patches, 
> but that can wait until a rewrite.

Modern XFS and EXT filesystems should not hit these code paths at all,
except for symbolic links, and even then only with GLOB_ONLYDIR.

> +# define dirfd(str) __dirfd (str)

This needs an #undef before it, else it causes build errors as glibc
already has a definition for dirfd() and it conflicts with this one.
Prudence says they should *all* be protected as such, but I only mention
the new one.

> +                  case DT_LNK: case DT_UNKNOWN:

I contemplated the case of symbolic links; I couldn't find anything in
the standards about it but I went with "glob does what shell wildcards
do" and those followed links, and I think that makes sense, so OK.  I
added that case to my local test area and it seems to do what I think
people will expect it to do.

Re: [patch v2] glob: resolve DT_UNKNOWN via is_dir

[Paul Eggert]

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

On 3/22/22 21:34, DJ Delorie wrote:

>> Of course performance will suffer with all these correctness patches,
>> but that can wait until a rewrite.
> 
> Modern XFS and EXT filesystems should not hit these code paths at all,
> except for symbolic links, and even then only with GLOB_ONLYDIR.

Right, though glob uses GLOB_ONLYDIR internally so this slower code will 
execute in some cases even when the user doesn't specify GLOB_ONLYDIR.


>> +# define dirfd(str) __dirfd (str)
> 
> This needs an #undef before it, else it causes build errors as glibc
> already has a definition for dirfd() and it conflicts with this one.
> Prudence says they should *all* be protected as such, but I only mention
> the new one.

Thanks, I protected it with an ifdef instead as that's more likely 
perform better.

Also, I noticed that the Gnulib glob module needs to be GPL not LGPLv2+, 
since it now depends on GPL modules like fstatat. So I fixed this 
(Gnulib-specific) issue as well.


> I contemplated the case of symbolic links; I couldn't find anything in
> the standards about it but I went with "glob does what shell wildcards
> do" and those followed links, and I think that makes sense, so OK.  I
> added that case to my local test area and it seems to do what I think
> people will expect it to do.

Thanks for checking that.

I installed the attached into Gnulib.  The first two patches are the 
same as what I sent earlier, except with the ifdef and GPL changes 
mentioned above. The 3rd patch tests for glibc bug 25659, replaces glibc 
glob if the bug is present, and makes sure the resulting glob passes a 
sanity check.

I hope this lets us make the Gnulib and glibc glob.c identical.

[-- Attachment #2: 0001-glob-resolve-DT_UNKNOWN-via-is_dir.patch --]
[-- Type: text/x-patch, Size: 3880 bytes --]

From ab8b3afda2aadcfd1ab7fd9f7bed92467dfe24af Mon Sep 17 00:00:00 2001
From: DJ Delorie <dj@redhat.com>
Date: Wed, 23 Mar 2022 09:39:37 -0700
Subject: [PATCH 1/3] glob: resolve DT_UNKNOWN via is_dir

The DT_* values returned by getdents (readdir) are only hints and
not required.  In fact, some Linux filesystems return DT_UNKNOWN
for most entries, regardless of actual type.  This causes make
to mis-match patterns with a trailing slash (via GLOB_ONLYDIR)
(see make's functions/wildcard test case).  Thus, this patch
detects that case and uses is_dir() to make the type known enough
for proper operation.

Performance in non-DT_UNKNOWN cases is not affected.

The lack of DT_* is a well known issue on older XFS installations
(for example, RHEL 7 and 8, Fedora 28) but can be recreated by
creating an XFS filesystem with flags that mimic older behavior:

$ fallocate -l 10G /xfs.fs
$ mkfs.xfs -n ftype=0 -m crc=0 -f /xfs.fs
$ mkdir /xfs
$ mount -o loop /xfs.fs /xfs
---
 ChangeLog  | 23 +++++++++++++++++++++++
 lib/glob.c | 28 +++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 5fab584974..8c50a52c78 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,26 @@
+2022-03-23  DJ Delorie  <dj@redhat.com>
+
+	glob: resolve DT_UNKNOWN via is_dir
+
+	The DT_* values returned by getdents (readdir) are only hints and
+	not required.  In fact, some Linux filesystems return DT_UNKNOWN
+	for most entries, regardless of actual type.  This causes make
+	to mis-match patterns with a trailing slash (via GLOB_ONLYDIR)
+	(see make's functions/wildcard test case).  Thus, this patch
+	detects that case and uses is_dir() to make the type known enough
+	for proper operation.
+
+	Performance in non-DT_UNKNOWN cases is not affected.
+
+	The lack of DT_* is a well known issue on older XFS installations
+	(for example, RHEL 7 and 8, Fedora 28) but can be recreated by
+	creating an XFS filesystem with flags that mimic older behavior:
+
+	$ fallocate -l 10G /xfs.fs
+	$ mkfs.xfs -n ftype=0 -m crc=0 -f /xfs.fs
+	$ mkdir /xfs
+	$ mount -o loop /xfs.fs /xfs
+
 2022-03-20  Jim Meyering  <meyering@fb.com>
 
 	maint: bootstrap: split a too-long line
diff --git a/lib/glob.c b/lib/glob.c
index f8d8a306f2..0da46ac138 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -1381,7 +1381,33 @@ 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;
+                  case DT_DIR: case DT_LNK: break;
+                  case DT_UNKNOWN:
+                    {
+                      /* The filesystem was too lazy to give us a hint,
+                         so we have to do it the hard way.  */
+                      char *fullpath, *p;
+                      bool isdir;
+                      int need = strlen (directory) + strlen (d.name) + 2;
+                      int use_alloca = glob_use_alloca (alloca_used, need);
+                      if (use_alloca)
+                        fullpath = alloca_account (need, alloca_used);
+                      else
+                        {
+                          fullpath = malloc (need);
+                          if (fullpath == NULL)
+                            goto memory_error;
+                        }
+                      p = stpcpy (fullpath, directory);
+                      *p++ = '/';
+                      strcpy (p, d.name);
+                      isdir = is_dir (fullpath, flags, pglob);
+                      if (!use_alloca)
+                        free (fullpath);
+                      if (isdir)
+                        break;
+                      continue;
+                    }
                   default: continue;
                   }
 
-- 
2.32.0


[-- Attachment #3: 0002-glob-fix-symlink-and-issues-improve-speed.patch --]
[-- Type: text/x-patch, Size: 9310 bytes --]

From 2f7f02986f9d338b5bb0e865bfd278678fb96325 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 23 Mar 2022 09:52:58 -0700
Subject: [PATCH 2/3] glob: fix symlink and // issues; improve speed

* lib/glob.c: Include fcntl.h.
(dirfd) [_LIBC]: New macro.
(GLOB_STAT64, GLOB_LSTAT64): Remove.  Replace all uses with ...
(GLOB_FSTATAT64): ... this new macro.
(glob_in_dir): Treat DT_LNK like DT_UNKNOWN.
Use directory-relative fstatat unless GLOB_ALTDIRFUNC, or dirfd fails.
Avoid duplicate strlen (directory).
Work even if directory is "/", without turning it into "//".
Use a scratch buffer instead of by-hand alloca stuff.
Use mempcpy and memcpy instead of stpcpy and strcpy.
* modules/glob (Depends-on): Add dirfd, fstatat.  Remove stat.
(License): Change from LGPLv2+ to GPL, since it depends on
fstatat.
---
 ChangeLog    | 17 ++++++++++++
 lib/glob.c   | 76 +++++++++++++++++++++++++++-------------------------
 modules/glob |  5 ++--
 3 files changed, 60 insertions(+), 38 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8c50a52c78..a0d3519162 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,20 @@
+2022-03-23  Paul Eggert  <eggert@cs.ucla.edu>
+
+	glob: fix symlink and // issues; improve speed
+	* lib/glob.c: Include fcntl.h.
+	(dirfd) [_LIBC]: New macro.
+	(GLOB_STAT64, GLOB_LSTAT64): Remove.  Replace all uses with ...
+	(GLOB_FSTATAT64): ... this new macro.
+	(glob_in_dir): Treat DT_LNK like DT_UNKNOWN.
+	Use directory-relative fstatat unless GLOB_ALTDIRFUNC, or dirfd fails.
+	Avoid duplicate strlen (directory).
+	Work even if directory is "/", without turning it into "//".
+	Use a scratch buffer instead of by-hand alloca stuff.
+	Use mempcpy and memcpy instead of stpcpy and strcpy.
+	* modules/glob (Depends-on): Add dirfd, fstatat.  Remove stat.
+	(License): Change from LGPLv2+ to GPL, since it depends on
+	fstatat.
+
 2022-03-23  DJ Delorie  <dj@redhat.com>
 
 	glob: resolve DT_UNKNOWN via is_dir
diff --git a/lib/glob.c b/lib/glob.c
index 0da46ac138..52c79b4cd8 100644
--- a/lib/glob.c
+++ b/lib/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,9 @@
 # define sysconf(id) __sysconf (id)
 # define closedir(dir) __closedir (dir)
 # define opendir(name) __opendir (name)
+# ifndef dirfd
+#  define dirfd(str) __dirfd (str)
+# endif
 # define readdir(str) __readdir64 (str)
 # define getpwnam_r(name, bufp, buf, len, res) \
     __getpwnam_r (name, bufp, buf, len, res)
@@ -69,11 +73,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 +89,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 +215,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 +258,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.  */
@@ -1283,6 +1285,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) };
@@ -1354,6 +1358,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;
@@ -1381,34 +1386,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: break;
-                  case DT_UNKNOWN:
-                    {
-                      /* The filesystem was too lazy to give us a hint,
-                         so we have to do it the hard way.  */
-                      char *fullpath, *p;
-                      bool isdir;
-                      int need = strlen (directory) + strlen (d.name) + 2;
-                      int use_alloca = glob_use_alloca (alloca_used, need);
-                      if (use_alloca)
-                        fullpath = alloca_account (need, alloca_used);
-                      else
-                        {
-                          fullpath = malloc (need);
-                          if (fullpath == NULL)
-                            goto memory_error;
-                        }
-                      p = stpcpy (fullpath, directory);
-                      *p++ = '/';
-                      strcpy (p, d.name);
-                      isdir = is_dir (fullpath, flags, pglob);
-                      if (!use_alloca)
-                        free (fullpath);
-                      if (isdir)
-                        break;
-                      continue;
-                    }
                   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)
@@ -1540,5 +1543,6 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
       __set_errno (save);
     }
 
+  scratch_buffer_free (&s);
   return result;
 }
diff --git a/modules/glob b/modules/glob
index ba270e8427..83cd729ceb 100644
--- a/modules/glob
+++ b/modules/glob
@@ -17,8 +17,10 @@ alloca          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 builtin-expect  [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 closedir        [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 d-type          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+dirfd           [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 flexmember      [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 fnmatch         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
+fstatat         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 getlogin_r      [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 libc-config     [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 memchr          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
@@ -26,7 +28,6 @@ mempcpy         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 opendir         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 readdir         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 scratch_buffer  [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
-stat            [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 stdbool         [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 stdint          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
 strdup          [test $HAVE_GLOB = 0 || test $REPLACE_GLOB = 1]
@@ -61,7 +62,7 @@ Link:
 $(LIB_MBRTOWC)
 
 License:
-LGPLv2+
+GPL
 
 Maintainer:
 all, glibc
-- 
2.32.0


[-- Attachment #4: 0003-glob-test-for-glibc-bug-25659.patch --]
[-- Type: text/x-patch, Size: 4155 bytes --]

From e265ddd557aa6f8d96919f87680c8525ae8fa84c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 23 Mar 2022 10:22:51 -0700
Subject: [PATCH 3/3] glob: test for glibc bug 25659

https://sourceware.org/bugzilla/show_bug.cgi?id=25659
* m4/glob.m4 (gl_GLOB): Replace glob if it has bug 25659.
* tests/test-glob.c (main): Test for glibc bug 25659.
---
 ChangeLog         |  5 +++++
 m4/glob.m4        | 41 ++++++++++++++++++++++++++++++++++++++++-
 tests/test-glob.c |  9 ++++++++-
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a0d3519162..7ea4f7797b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
 2022-03-23  Paul Eggert  <eggert@cs.ucla.edu>
 
+	glob: test for glibc bug 25659
+	https://sourceware.org/bugzilla/show_bug.cgi?id=25659
+	* m4/glob.m4 (gl_GLOB): Replace glob if it has bug 25659.
+	* tests/test-glob.c (main): Test for glibc bug 25659.
+
 	glob: fix symlink and // issues; improve speed
 	* lib/glob.c: Include fcntl.h.
 	(dirfd) [_LIBC]: New macro.
diff --git a/m4/glob.m4 b/m4/glob.m4
index 0d1426389b..cf5f93930c 100644
--- a/m4/glob.m4
+++ b/m4/glob.m4
@@ -1,4 +1,4 @@
-# glob.m4 serial 24
+# glob.m4 serial 25
 dnl Copyright (C) 2005-2007, 2009-2022 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -66,6 +66,45 @@ char a[_GNU_GLOB_INTERFACE_VERSION == 1 || _GNU_GLOB_INTERFACE_VERSION == 2 ? 1
       esac
     fi
 
+    if test $REPLACE_GLOB = 0; then
+      AC_CACHE_CHECK([whether glob NOTDIR*/ omits symlink to nondir],
+                     [gl_cv_glob_omit_nondir_symlinks],
+        [if test $cross_compiling != yes; then
+           if ln -s conf-file conf$$-globtest 2>/dev/null && touch conf-file
+           then
+             gl_cv_glob_omit_nondir_symlinks=maybe
+           else
+             # If we can't make a symlink, then we cannot test this issue.  Be
+             # pessimistic about this.
+             gl_cv_glob_omit_nondir_symlinks=no
+           fi
+           if test $gl_cv_glob_omit_nondir_symlinks = maybe; then
+             AC_RUN_IFELSE(
+               [AC_LANG_PROGRAM(
+                  [[#include <stddef.h>
+                    #include <glob.h>]],
+                  [[glob_t found;
+                    if (glob ("conf*-globtest/", 0, NULL, &found) != GLOB_NOMATCH)
+                      return 1;
+                    globfree (&found);
+                  ]])],
+               [gl_cv_glob_omit_nondir_symlinks=yes],
+               [gl_cv_glob_omit_nondir_symlinks=no],
+               [dnl We don't get here.
+                :
+               ])
+           fi
+           rm -f conf$$-globtest
+         else
+           gl_cv_glob_omit_nondir_symlinks="$gl_cross_guess_normal"
+         fi
+        ])
+      case "$gl_cv_glob_omit_nondir_symlinks" in
+        *yes) ;;
+        *) REPLACE_GLOB=1 ;;
+      esac
+    fi
+
   fi
 
   if test $ac_cv_func_glob_pattern_p = no; then
diff --git a/tests/test-glob.c b/tests/test-glob.c
index 2709080bc7..568acf14b7 100644
--- a/tests/test-glob.c
+++ b/tests/test-glob.c
@@ -72,7 +72,9 @@ main ()
   globfree (&g);
 
   if ((symlink (GL_NO_SUCH_FILE, BASE "globlink1") == 0 || errno == EEXIST)
-      && (symlink (".", BASE "globlink2") == 0 || errno == EEXIST))
+      && (symlink (".", BASE "globlink2") == 0 || errno == EEXIST)
+      && (symlink (BASE "globfile", BASE "globlink3") == 0 || errno == EEXIST)
+      && close (creat (BASE "globfile", 0666)) == 0)
     {
       res = glob (BASE "globlink[12]", 0, NULL, &g);
       ASSERT (res == 0 && g.gl_pathc == 2);
@@ -80,6 +82,11 @@ main ()
       ASSERT (strcmp (g.gl_pathv[1], BASE "globlink2") == 0);
       globfree (&g);
 
+      res = glob (BASE "globlink[123]/", 0, NULL, &g);
+      ASSERT (res == 0 && g.gl_pathc == 1);
+      ASSERT (strcmp (g.gl_pathv[0], BASE "globlink2/") == 0);
+      globfree (&g);
+
       res = glob (BASE "globlink[12]", GLOB_MARK, NULL, &g);
       ASSERT (res == 0 && g.gl_pathc == 2);
       ASSERT (strcmp (g.gl_pathv[0], BASE "globlink1") == 0);
-- 
2.32.0

Re: [patch v2] glob: resolve DT_UNKNOWN via is_dir

[DJ Delorie]

Paul Eggert <eggert@cs.ucla.edu> writes:
>>> +# define dirfd(str) __dirfd (str)
>> 
>> This needs an #undef before it, else it causes build errors as glibc
>> already has a definition for dirfd() and it conflicts with this one.
>> Prudence says they should *all* be protected as such, but I only mention
>> the new one.
>
> Thanks, I protected it with an ifdef instead as that's more likely 
> perform better.

The use in glob.c conflicts with the definition in glibc, so #undef is
needed, not #ifndef.  glibc has:

  /* This is the data type of directory stream objects.
     The actual structure is opaque to users.  */
  typedef struct __dirstream DIR;
  extern int dirfd (DIR *__dirp) __THROW __nonnull ((1));

What works in a glibc build is this:

  # undef dirfd
  # define dirfd(str) __dirfd (str)

Re: [patch v2] glob: resolve DT_UNKNOWN via is_dir

[Bruno Haible]

Paul Eggert wrote on 2022-03-23:
> The 3rd patch tests for glibc bug 25659 ...

This patch produces a warning

../../gltests/test-glob.c: In function 'main':
../../gltests/test-glob.c:77:17: warning: implicit declaration of function 'creat' [-Wimplicit-function-declaration]
   77 |       && close (creat (BASE "globfile", 0666)) == 0)
      |                 ^~~~~

Fixed as follows.


2022-05-14  Bruno Haible  <bruno@clisp.org>

	glob tests: Fix a warning (regression from 2022-03-23).
	* tests/test-glob.c: Include <fcntl.h>.
	* modules/glob-tests (Depends-on): Add fcntl-h.

diff --git a/modules/glob-tests b/modules/glob-tests
index ec519cf38d..afce2dbca0 100644
--- a/modules/glob-tests
+++ b/modules/glob-tests
@@ -4,6 +4,7 @@ tests/signature.h
 tests/macros.h
 
 Depends-on:
+fcntl-h
 symlink
 
 configure.ac:
diff --git a/tests/test-glob.c b/tests/test-glob.c
index 568acf14b7..61c7ca4583 100644
--- a/tests/test-glob.c
+++ b/tests/test-glob.c
@@ -26,6 +26,7 @@ SIGNATURE_CHECK (glob, int, (char const *, int, int (*) (char const *, int),
 SIGNATURE_CHECK (globfree, void, (glob_t *));
 
 #include <errno.h>
+#include <fcntl.h>
 #include <string.h>
 #include <unistd.h>