public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 2/9] posix: accept inode 0 is a valid inode number (BZ #19971)
  2017-09-05 20:25 [PATCH 0/9] posix: glob fixes and refactor Adhemerval Zanella
                   ` (4 preceding siblings ...)
  2017-09-05 20:25 ` [PATCH 7/9] posix: Consolidate glob implementation Adhemerval Zanella
@ 2017-09-05 20:25 ` Adhemerval Zanella
  2017-09-05 20:25 ` [PATCH 8/9] posix: Use enum for __glob_pattern_type result Adhemerval Zanella
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-05 20:25 UTC (permalink / raw)
  To: libc-alpha; +Cc: Paul Eggert

According to this kernel commit 2adc376c55194, d_ino 0 is a regular inode
number on Linux (which also matches POSIX, as it does not treat the value
as special).  This patch makes glob accept is a valid inode number.

This is also a sync with gnulib commit c8e57c1.

Checked on x86_64-linux-gnu and on a build using build-many-glibcs.py
for all major architectures.

	[BZ #1062]
	[BZ #19971]
	* posix/glob.c (struct readdir_result): Remove skip_entry member.
	(readdir_result_skip_entry, D_INO_TO_RESULT): Remove.
	All uses removed.
---
 ChangeLog    |  6 ++++++
 posix/glob.c | 21 ---------------------
 2 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/posix/glob.c b/posix/glob.c
index e19aa6f..36d9e5f 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -114,9 +114,6 @@ struct readdir_result
 #if defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE
   dirent_type type;
 #endif
-#if defined _LIBC || defined D_INO_IN_DIRENT
-  bool skip_entry;
-#endif
 };
 
 /* Initialize and return type member of struct readdir_result.  */
@@ -132,28 +129,12 @@ readdir_result_type (struct readdir_result d)
 #endif
 }
 
-/* Initialize and return skip_entry member of struct readdir_result.  */
-static bool
-readdir_result_skip_entry (struct readdir_result d)
-{
-/* Initializer for skip_entry.  POSIX does not require that the d_ino
-   field be present, and some systems do not provide it. */
-#if defined _LIBC || defined D_INO_IN_DIRENT
-# define D_INO_TO_RESULT(source) (source)->d_ino == 0,
-  return d.skip_entry;
-#else
-# define D_INO_TO_RESULT(source)
-  return false;
-#endif
-}
-
 /* Construct an initializer for a struct readdir_result object from a
    struct dirent *.  No copy of the name is made.  */
 #define READDIR_RESULT_INITIALIZER(source) \
   {					   \
     source->d_name,			   \
     D_TYPE_TO_RESULT (source)		   \
-    D_INO_TO_RESULT (source)		   \
   }
 
 /* Call gl_readdir on STREAM.  This macro can be overridden to reduce
@@ -1543,8 +1524,6 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 	      }
 	      if (d.name == NULL)
 		break;
-	      if (readdir_result_skip_entry (d))
-		continue;
 
 	      /* If we shall match only directories use the information
 		 provided by the dirent call if possible.  */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 6/9] posix: fix glob bugs with long login names
  2017-09-05 20:25 [PATCH 0/9] posix: glob fixes and refactor Adhemerval Zanella
  2017-09-05 20:25 ` [PATCH 9/9] posix: Fix glob with GLOB_NOCHECK returning modified patterns (BZ#10246) Adhemerval Zanella
  2017-09-05 20:25 ` [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866] Adhemerval Zanella
@ 2017-09-05 20:25 ` Adhemerval Zanella
  2017-09-05 20:25 ` [PATCH 1/9] posix: Sync glob with gnulib [BZ #1062] Adhemerval Zanella
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-05 20:25 UTC (permalink / raw)
  To: libc-alpha; +Cc: Paul Eggert

Current glob implementation allows unlimited user name for home
directory construction on GLOB_TILDE case.  To accomplish it glob
either construct a name on stack if size are small enough (based
on current alloca_used) or in heap otherwise.

This patch simplifies storage allocation by using the same scratch
buffer for both get_rlogin_r and getpwnam_r.

This also syncs with gnulib commit 064df0b (glob: fix bugs with long
login names).

Checked on x86_64-linux-gnu and on a build using build-many-glibcs.py
for all major architectures.

	* posix/glob.c (GET_LOGIN_NAME_MAX): Remove.
	(glob): Use the same scratch buffer for both getlogin_r and
	getpwnam_r.  Don’t require preallocation of the login name.  This
	simplifies storage allocation, and corrects the handling of
	long login names.
---
 ChangeLog    |  7 +++++
 posix/glob.c | 88 +++++++++++++++++++++---------------------------------------
 2 files changed, 37 insertions(+), 58 deletions(-)

diff --git a/posix/glob.c b/posix/glob.c
index 340cf08..2c8a3dc 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -75,12 +75,6 @@
 #include <flexmember.h>
 #include <glob_internal.h>
 #include <scratch_buffer.h>
-
-#ifdef _SC_LOGIN_NAME_MAX
-# define GET_LOGIN_NAME_MAX()	sysconf (_SC_LOGIN_NAME_MAX)
-#else
-# define GET_LOGIN_NAME_MAX()	(-1)
-#endif
 \f
 static const char *next_brace_sub (const char *begin, int flags) __THROWNL;
 
@@ -611,67 +605,45 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      else
 		home_dir = "c:/users/default"; /* poor default */
 #else
-	      int success;
-	      char *name;
-	      int malloc_name = 0;
-	      size_t buflen = GET_LOGIN_NAME_MAX () + 1;
-
-	      if (buflen == 0)
-		/* 'sysconf' does not support _SC_LOGIN_NAME_MAX.  Try
-		   a moderate value.  */
-		buflen = 20;
-	      if (glob_use_alloca (alloca_used, buflen))
-		name = alloca_account (buflen, alloca_used);
-	      else
+	      int err;
+	      struct passwd *p;
+	      struct passwd pwbuf;
+	      struct scratch_buffer s;
+	      scratch_buffer_init (&s);
+	      while (true)
 		{
-		  name = malloc (buflen);
-		  if (name == NULL)
+		  p = NULL;
+		  err = __getlogin_r (s.data, s.length);
+		  if (err == 0)
 		    {
-		      retval = GLOB_NOSPACE;
-		      goto out;
-		    }
-		  malloc_name = 1;
-		}
-
-	      success = __getlogin_r (name, buflen) == 0;
-	      if (success)
-		{
-		  struct passwd *p;
-		  struct scratch_buffer pwtmpbuf;
-		  scratch_buffer_init (&pwtmpbuf);
 # if defined HAVE_GETPWNAM_R || defined _LIBC
-		  struct passwd pwbuf;
-
-		  while (getpwnam_r (name, &pwbuf,
-				     pwtmpbuf.data, pwtmpbuf.length, &p)
-			 == ERANGE)
-		    {
-		      if (!scratch_buffer_grow (&pwtmpbuf))
-			{
-			  retval = GLOB_NOSPACE;
-			  goto out;
-			}
-		    }
+		      size_t ssize = strlen (s.data) + 1;
+		      err = getpwnam_r (s.data, &pwbuf, s.data + ssize,
+					s.length - ssize, &p);
 # else
-		  p = getpwnam (name);
+		      p = getpwnam (s.data);
+		      if (p == NULL)
+			err = errno;
 # endif
-		  if (p != NULL)
+		    }
+		  if (err != ERANGE)
+		    break;
+		  if (!scratch_buffer_grow (&s))
 		    {
-		      home_dir = strdup (p->pw_dir);
-		      malloc_home_dir = 1;
-		      if (home_dir == NULL)
-			{
-			  scratch_buffer_free (&pwtmpbuf);
-			  retval = GLOB_NOSPACE;
-			  goto out;
-			}
+		      retval = GLOB_NOSPACE;
+		      goto out;
 		    }
-		  scratch_buffer_free (&pwtmpbuf);
 		}
-	      else
+	      if (err == 0)
+		{
+		  home_dir = strdup (p->pw_dir);
+		  malloc_home_dir = 1;
+		}
+	      scratch_buffer_free (&s);
+	      if (err == 0 && home_dir == NULL)
 		{
-		  if (__glibc_unlikely (malloc_name))
-		    free (name);
+		  retval = GLOB_NOSPACE;
+		  goto out;
 		}
 #endif /* WINDOWS32 */
 	    }
-- 
2.7.4

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 8/9] posix: Use enum for __glob_pattern_type result
  2017-09-05 20:25 [PATCH 0/9] posix: glob fixes and refactor Adhemerval Zanella
                   ` (5 preceding siblings ...)
  2017-09-05 20:25 ` [PATCH 2/9] posix: accept inode 0 is a valid inode number (BZ #19971) Adhemerval Zanella
@ 2017-09-05 20:25 ` Adhemerval Zanella
  2017-09-06  1:30   ` Paul Eggert
  2017-09-06  4:18   ` Paul Eggert
  2017-09-05 20:25 ` [PATCH 4/9] Sync scratch_buffer with gnulib Adhemerval Zanella
  2017-09-05 20:25 ` [PATCH 5/9] posix: Fix getpwnam_r usage (BZ #1062) Adhemerval Zanella
  8 siblings, 2 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-05 20:25 UTC (permalink / raw)
  To: libc-alpha; +Cc: Paul Eggert

This patch replaces the internal integer constant from
__glob_pattern_type return with a proper enum.

Checked on x86_64-linux-gnu and on a build using build-many-glibcs.py
for all major architectures.

	* posix/glob_internal.h (glob_pattern_type_t): New enumeration.
	(__glob_pattern_type): Use __glob_pat_types.
	* posix/glob_pattern_p.c (__glob_pattern_p): Likewise.
	* posix/glob.c (glob): Likewise.
	(glob_in_dir): Likewise.
---
 ChangeLog              |  6 ++++++
 posix/glob.c           |  8 ++++----
 posix/glob_internal.h  | 18 +++++++++++++-----
 posix/glob_pattern_p.c |  2 +-
 4 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/posix/glob.c b/posix/glob.c
index 2c8a3dc..30a4143 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -903,7 +903,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
      [ which we handle the same, using fnmatch.  Broken unterminated
      pattern bracket expressions ought to be rare enough that it is
      not worth special casing them, fnmatch will do the right thing.  */
-  if (meta & 5)
+  if (meta & (__GLOB_SPECIAL | __GLOB_BRACKET))
     {
       /* The directory name contains metacharacters, so we
 	 have to glob for the directory, and then glob for
@@ -1044,7 +1044,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
       size_t old_pathc = pglob->gl_pathc;
       int orig_flags = flags;
 
-      if (meta & 2)
+      if (meta & __GLOB_BACKSLASH)
 	{
 	  char *p = strchr (dirname, '\\'), *q;
 	  /* We need to unescape the dirname string.  It is certainly
@@ -1242,14 +1242,14 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
                        / sizeof init_names->name[0]);
 
   meta = __glob_pattern_type (pattern, !(flags & GLOB_NOESCAPE));
-  if (meta == 0 && (flags & (GLOB_NOCHECK|GLOB_NOMAGIC)))
+  if (meta == __GLOB_NONE && (flags & (GLOB_NOCHECK|GLOB_NOMAGIC)))
     {
       /* We need not do any tests.  The PATTERN contains no meta
 	 characters and we must not return an error therefore the
 	 result will always contain exactly one name.  */
       flags |= GLOB_NOCHECK;
     }
-  else if (meta == 0)
+  else if (meta == __GLOB_NONE)
     {
       union
       {
diff --git a/posix/glob_internal.h b/posix/glob_internal.h
index 12c9366..2e09132 100644
--- a/posix/glob_internal.h
+++ b/posix/glob_internal.h
@@ -19,35 +19,43 @@
 #ifndef GLOB_INTERNAL_H
 # define GLOB_INTERNAL_H
 
+enum glob_pattern_type_t
+{
+  __GLOB_NONE      = 0x0,
+  __GLOB_SPECIAL   = 0x1,
+  __GLOB_BACKSLASH = 0x2,
+  __GLOB_BRACKET   = 0x4 
+};
+
 static inline int
 __glob_pattern_type (const char *pattern, int quote)
 {
   const char *p;
-  int ret = 0;
+  int ret = __GLOB_NONE;
 
   for (p = pattern; *p != '\0'; ++p)
     switch (*p)
       {
       case '?':
       case '*':
-        return 1;
+        return __GLOB_SPECIAL;
 
       case '\\':
         if (quote)
           {
             if (p[1] != '\0')
               ++p;
-            ret |= 2;
+            ret |= __GLOB_BACKSLASH;
           }
         break;
 
       case '[':
-        ret |= 4;
+        ret |= __GLOB_BRACKET;
         break;
 
       case ']':
         if (ret & 4)
-          return 1;
+          return __GLOB_SPECIAL;
         break;
       }
 
diff --git a/posix/glob_pattern_p.c b/posix/glob_pattern_p.c
index a17d337..2502772 100644
--- a/posix/glob_pattern_p.c
+++ b/posix/glob_pattern_p.c
@@ -28,6 +28,6 @@
 int
 __glob_pattern_p (const char *pattern, int quote)
 {
-  return __glob_pattern_type (pattern, quote) == 1;
+  return __glob_pattern_type (pattern, quote) == __GLOB_SPECIAL;
 }
 weak_alias (__glob_pattern_p, glob_pattern_p)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-05 20:25 [PATCH 0/9] posix: glob fixes and refactor Adhemerval Zanella
  2017-09-05 20:25 ` [PATCH 9/9] posix: Fix glob with GLOB_NOCHECK returning modified patterns (BZ#10246) Adhemerval Zanella
@ 2017-09-05 20:25 ` Adhemerval Zanella
  2017-09-06  1:27   ` Paul Eggert
  2017-09-09  9:50   ` Andreas Schwab
  2017-09-05 20:25 ` [PATCH 6/9] posix: fix glob bugs with long login names Adhemerval Zanella
                   ` (6 subsequent siblings)
  8 siblings, 2 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-05 20:25 UTC (permalink / raw)
  To: libc-alpha; +Cc: Paul Eggert

This patch makes glob match dangling symlinks.  Compared to other glob
implementation (*BSD, bash, musl, and other shells as well), GLIBC seems
the be the only one that does not match dangling symlinks.  As for
comment #5 in BZ #866, POSIX does not have any strict specification for
dangling symlinks match and it is reasonable that trying to glob everything
in a path should return all types of files (such as for a 'rm *').  Also,
comment #7 shows even more example where GLIBC current behavior is
unexepected.

I avoided adding another GNU specific flag to set this behavior and
instead make it the default.  Although this change the semanthic from
previous implementation, I think adding another compat symbol to be
really unecessary as from aforementioned reasons (current behavior not
defined in any standard, general idea of different implementation is
to list dangling symbols).

This also sync glob with gnulib commit fd1daf4 (glob: match dangling
symlinks).

Checked on x86_64-linux-gnu and on a build using build-many-glibcs.py
for all major architectures.

	[BZ #866]
	[BZ #1062]
	* posix/Makefile (tests): Remove bug-glob1 and tst-glob_symlinks.
	* posix/bug-glob1.c: Remove file.
	* posix/tst-glob_symlinks.c: New file.
	* lib/glob.c (__lstat64): New macro.
	(is_dir): New function.
	(glob, glob_in_dir): Match symlinks even if they are dangling.
	(link_stat, link_exists_p): Remove.  All uses removed.
---
 ChangeLog                 |  10 ++
 posix/Makefile            |   4 +-
 posix/bug-glob1.c         |  88 -----------------
 posix/glob.c              | 239 +++++++++++++++++-----------------------------
 posix/tst-glob_symlinks.c | 132 +++++++++++++++++++++++++
 5 files changed, 230 insertions(+), 243 deletions(-)
 delete mode 100644 posix/bug-glob1.c
 create mode 100644 posix/tst-glob_symlinks.c

diff --git a/posix/Makefile b/posix/Makefile
index 0ff8f5c..7188cba 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -79,7 +79,7 @@ tests		:= test-errno tstgetopt testfnm runtests runptests \
 		   tst-nice tst-nanosleep tst-regex2 \
 		   transbug tst-rxspencer tst-pcre tst-boost \
 		   bug-ga1 tst-vfork1 tst-vfork2 tst-vfork3 tst-waitid \
-		   tst-getaddrinfo2 bug-glob1 bug-glob2 bug-glob3 tst-sysconf \
+		   tst-getaddrinfo2 bug-glob2 bug-glob3 tst-sysconf \
 		   tst-execvp1 tst-execvp2 tst-execlp1 tst-execlp2 \
 		   tst-execv1 tst-execv2 tst-execl1 tst-execl2 \
 		   tst-execve1 tst-execve2 tst-execle1 tst-execle2 \
@@ -93,7 +93,7 @@ tests		:= test-errno tstgetopt testfnm runtests runptests \
 		   tst-fnmatch3 bug-regex36 tst-getaddrinfo5 \
 		   tst-posix_spawn-fd tst-posix_spawn-setsid \
 		   tst-posix_fadvise tst-posix_fadvise64 \
-		   tst-sysconf-empty-chroot
+		   tst-sysconf-empty-chroot tst-glob_symlinks
 tests-internal	:= bug-regex5 bug-regex20 bug-regex33 \
 		   tst-rfc3484 tst-rfc3484-2 tst-rfc3484-3
 xtests		:= bug-ga2
diff --git a/posix/bug-glob1.c b/posix/bug-glob1.c
deleted file mode 100644
index 05c2da7..0000000
--- a/posix/bug-glob1.c
+++ /dev/null
@@ -1,88 +0,0 @@
-/* Test case for globbing dangling symlink.  By Ulrich Drepper.  */
-#include <errno.h>
-#include <error.h>
-#include <glob.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-
-
-static void prepare (int argc, char *argv[]);
-#define PREPARE prepare
-static int do_test (void);
-#define TEST_FUNCTION do_test ()
-
-#include "../test-skeleton.c"
-
-
-static char *fname;
-
-static void
-prepare (int argc, char *argv[])
-{
-  if (argc < 2)
-    error (EXIT_FAILURE, 0, "missing argument");
-
-  size_t len = strlen (argv[1]);
-  static const char ext[] = "globXXXXXX";
-  fname = malloc (len + sizeof (ext));
-  if (fname == NULL)
-    error (EXIT_FAILURE, errno, "cannot create temp file");
- again:
-  strcpy (stpcpy (fname, argv[1]), ext);
-  fname = mktemp (fname);
-  if (fname == NULL || *fname == '\0')
-    error (EXIT_FAILURE, errno, "cannot create temp file name");
-  if (symlink ("bug-glob1-does-not-exist", fname) != 0)
-    {
-      if (errno == EEXIST)
-	goto again;
-
-      error (EXIT_FAILURE, errno, "cannot create symlink");
-    }
-  add_temp_file (fname);
-}
-
-
-static int
-do_test (void)
-{
-  glob_t gl;
-  int retval = 0;
-  int e;
-
-  e = glob (fname, 0, NULL, &gl);
-  if (e == 0)
-    {
-      printf ("glob(\"%s\") succeeded\n", fname);
-      retval = 1;
-    }
-  globfree (&gl);
-
-  size_t fnamelen = strlen (fname);
-  char buf[fnamelen + 2];
-
-  strcpy (buf, fname);
-  buf[fnamelen - 1] = '?';
-  e = glob (buf, 0, NULL, &gl);
-  if (e == 0)
-    {
-      printf ("glob(\"%s\") succeeded\n", buf);
-      retval = 1;
-    }
-  globfree (&gl);
-
-  strcpy (buf, fname);
-  buf[fnamelen] = '*';
-  buf[fnamelen + 1] = '\0';
-  e = glob (buf, 0, NULL, &gl);
-  if (e == 0)
-    {
-      printf ("glob(\"%s\") succeeded\n", buf);
-      retval = 1;
-    }
-  globfree (&gl);
-
-  return retval;
-}
diff --git a/posix/glob.c b/posix/glob.c
index 36d9e5f..15c6295 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -57,6 +57,9 @@
 # define readdir(str) __readdir64 (str)
 # define getpwnam_r(name, bufp, buf, len, res) \
     __getpwnam_r (name, bufp, buf, len, res)
+# ifndef __lstat64
+#  define __lstat64(fname, buf) __lxstat64 (_STAT_VER, fname, buf)
+# endif
 # ifndef __stat64
 #  define __stat64(fname, buf) __xstat64 (_STAT_VER, fname, buf)
 # endif
@@ -64,6 +67,7 @@
 # define FLEXIBLE_ARRAY_MEMBER
 #else /* !_LIBC */
 # define __getlogin_r(buf, len) getlogin_r (buf, len)
+# define __lstat64(fname, buf)  lstat (fname, buf)
 # define __stat64(fname, buf)   stat (fname, buf)
 # define __fxstatat64(_, d, f, st, flag) fstatat (d, f, st, flag)
 # define struct_stat64          struct stat
@@ -227,6 +231,18 @@ static int prefix_array (const char *prefix, char **array, size_t n) __THROWNL;
 static int collated_compare (const void *, const void *) __THROWNL;
 
 
+/* Return true if FILENAME is a directory or a symbolic link to a directory.
+   Use FLAGS and PGLOB to resolve the filename.  */
+static bool
+is_dir (char const *filename, int flags, glob_t const *pglob)
+{
+  struct stat st;
+  struct_stat64 st64;
+  return (__glibc_unlikely (flags & GLOB_ALTDIRFUNC)
+          ? pglob->gl_stat (filename, &st) == 0 && S_ISDIR (st.st_mode)
+          : __stat64 (filename, &st64) == 0 && S_ISDIR (st64.st_mode));
+}
+
 /* Find the end of the sub-pattern in a brace expression.  */
 static const char *
 next_brace_sub (const char *cp, int flags)
@@ -976,68 +992,53 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
      can give the answer now.  */
   if (filename == NULL)
     {
-      struct stat st;
-      struct_stat64 st64;
-
-      /* Return the directory if we don't check for error or if it exists.  */
-      if ((flags & GLOB_NOCHECK)
-	  || (((__builtin_expect (flags & GLOB_ALTDIRFUNC, 0))
-	       ? ((*pglob->gl_stat) (dirname, &st) == 0
-		  && S_ISDIR (st.st_mode))
-	       : (__stat64 (dirname, &st64) == 0 && S_ISDIR (st64.st_mode)))))
-	{
-	  size_t newcount = pglob->gl_pathc + pglob->gl_offs;
-	  char **new_gl_pathv;
+	size_t newcount = pglob->gl_pathc + pglob->gl_offs;
+	char **new_gl_pathv;
 
-	  if (newcount > SIZE_MAX / sizeof (char *) - 2)
-	    {
-	    nospace:
-	      free (pglob->gl_pathv);
-	      pglob->gl_pathv = NULL;
-	      pglob->gl_pathc = 0;
-	      retval = GLOB_NOSPACE;
-	      goto out;
-	    }
-
-	  new_gl_pathv = realloc (pglob->gl_pathv,
-				  (newcount + 2) * sizeof (char *));
-	  if (new_gl_pathv == NULL)
-	    goto nospace;
-	  pglob->gl_pathv = new_gl_pathv;
+	if (newcount > SIZE_MAX / sizeof (char *) - 2)
+	  {
+	  nospace:
+	    free (pglob->gl_pathv);
+	    pglob->gl_pathv = NULL;
+	    pglob->gl_pathc = 0;
+	    retval = GLOB_NOSPACE;
+	    goto out;
+	  }
 
-	  if (flags & GLOB_MARK)
-	    {
-	      char *p;
-	      pglob->gl_pathv[newcount] = malloc (dirlen + 2);
-	      if (pglob->gl_pathv[newcount] == NULL)
-		goto nospace;
-	      p = mempcpy (pglob->gl_pathv[newcount], dirname, dirlen);
-	      p[0] = '/';
-	      p[1] = '\0';
-	      if (__glibc_unlikely (malloc_dirname))
-		free (dirname);
-	    }
-	  else
-	    {
-	      if (__glibc_unlikely (malloc_dirname))
-		pglob->gl_pathv[newcount] = dirname;
-	      else
-		{
-		  pglob->gl_pathv[newcount] = strdup (dirname);
-		  if (pglob->gl_pathv[newcount] == NULL)
-		    goto nospace;
-		}
-	    }
-	  pglob->gl_pathv[++newcount] = NULL;
-	  ++pglob->gl_pathc;
-	  pglob->gl_flags = flags;
+	new_gl_pathv = realloc (pglob->gl_pathv,
+				(newcount + 2) * sizeof (char *));
+	if (new_gl_pathv == NULL)
+	  goto nospace;
+	pglob->gl_pathv = new_gl_pathv;
 
-	  return 0;
-	}
+	if (flags & GLOB_MARK && is_dir (dirname, flags, pglob))
+	  {
+	    char *p;
+	    pglob->gl_pathv[newcount] = malloc (dirlen + 2);
+	    if (pglob->gl_pathv[newcount] == NULL)
+	      goto nospace;
+	    p = mempcpy (pglob->gl_pathv[newcount], dirname, dirlen);
+	    p[0] = '/';
+	    p[1] = '\0';
+	    if (__glibc_unlikely (malloc_dirname))
+	      free (dirname);
+	  }
+	else
+	  {
+	    if (__glibc_unlikely (malloc_dirname))
+	      pglob->gl_pathv[newcount] = dirname;
+	    else
+	      {
+		pglob->gl_pathv[newcount] = strdup (dirname);
+		if (pglob->gl_pathv[newcount] == NULL)
+		  goto nospace;
+	      }
+	  }
+	pglob->gl_pathv[++newcount] = NULL;
+	++pglob->gl_pathc;
+	pglob->gl_flags = flags;
 
-      /* Not found.  */
-      retval = GLOB_NOMATCH;
-      goto out;
+	return 0;
     }
 
   meta = __glob_pattern_type (dirname, !(flags & GLOB_NOESCAPE));
@@ -1245,15 +1246,9 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
     {
       /* Append slashes to directory names.  */
       size_t i;
-      struct stat st;
-      struct_stat64 st64;
 
       for (i = oldcount; i < pglob->gl_pathc + pglob->gl_offs; ++i)
-	if ((__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
-	     ? ((*pglob->gl_stat) (pglob->gl_pathv[i], &st) == 0
-		&& S_ISDIR (st.st_mode))
-	     : (__stat64 (pglob->gl_pathv[i], &st64) == 0
-		&& S_ISDIR (st64.st_mode))))
+	if (is_dir (pglob->gl_pathv[i], flags, pglob))
 	  {
 	    size_t len = strlen (pglob->gl_pathv[i]) + 2;
 	    char *new = realloc (pglob->gl_pathv[i], len);
@@ -1359,56 +1354,6 @@ prefix_array (const char *dirname, char **array, size_t n)
   return 0;
 }
 
-/* We put this in a separate function mainly to allow the memory
-   allocated with alloca to be recycled.  */
-static int
-__attribute_noinline__
-link_stat (const char *dir, size_t dirlen, const char *fname,
-	   glob_t *pglob
-# if !defined _LIBC && !HAVE_FSTATAT
-	   , int flags
-# endif
-	   )
-{
-  size_t fnamelen = strlen (fname);
-  char *fullname = __alloca (dirlen + 1 + fnamelen + 1);
-  struct stat st;
-
-  mempcpy (mempcpy (mempcpy (fullname, dir, dirlen), "/", 1),
-	   fname, fnamelen + 1);
-
-# if !defined _LIBC && !HAVE_FSTATAT
-  if (__builtin_expect ((flags & GLOB_ALTDIRFUNC) == 0, 1))
-    {
-      struct_stat64 st64;
-      return __stat64 (fullname, &st64);
-    }
-# endif
-  return (*pglob->gl_stat) (fullname, &st);
-}
-
-/* Return true if DIR/FNAME exists.  */
-static int
-link_exists_p (int dfd, const char *dir, size_t dirlen, const char *fname,
-	       glob_t *pglob, int flags)
-{
-  int status;
-# if defined _LIBC || HAVE_FSTATAT
-  if (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0))
-    status = link_stat (dir, dirlen, fname, pglob);
-  else
-    {
-      /* dfd cannot be -1 here, because dirfd never returns -1 on
-	 glibc, or on hosts that have fstatat.  */
-      struct_stat64 st64;
-      status = __fxstatat64 (_STAT_VER, dfd, fname, &st64, 0);
-    }
-# else
-  status = link_stat (dir, dirlen, fname, pglob, flags);
-# endif
-  return status == 0 || errno == EOVERFLOW;
-}
-
 /* Like 'glob', but PATTERN is a final pathname component,
    and matches are searched for in DIRECTORY.
    The GLOB_NOSORT bit in FLAGS is ignored.  No sorting is ever done.
@@ -1450,8 +1395,6 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
     }
   else if (meta == 0)
     {
-      /* Since we use the normal file functions we can also use stat()
-	 to verify the file is there.  */
       union
       {
 	struct stat st;
@@ -1476,8 +1419,8 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 			"/", 1),
 	       pattern, patlen + 1);
       if (((__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
-	   ? (*pglob->gl_stat) (fullname, &ust.st)
-	    : __stat64 (fullname, &ust.st64))
+	   ? (*pglob->gl_lstat) (fullname, &ust.st)
+	    : __lstat64 (fullname, &ust.st64))
 	   == 0)
 	  || errno == EOVERFLOW)
 	/* We found this file to be existing.  Now tell the rest
@@ -1501,8 +1444,6 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 	}
       else
 	{
-	  int dfd = (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
-		     ? -1 : dirfd ((DIR *) stream));
 	  int fnm_flags = ((!(flags & GLOB_PERIOD) ? FNM_PERIOD : 0)
 			   | ((flags & GLOB_NOESCAPE) ? FNM_NOESCAPE : 0));
 	  flags |= GLOB_MAGCHAR;
@@ -1536,42 +1477,34 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 
 	      if (fnmatch (pattern, d.name, fnm_flags) == 0)
 		{
-		  /* If the file we found is a symlink we have to
-		     make sure the target file exists.  */
-		  dirent_type type = readdir_result_type (d);
-		  if (! (type == DT_LNK || type == DT_UNKNOWN)
-		      || link_exists_p (dfd, directory, dirlen, d.name,
-					pglob, flags))
+		  if (cur == names->count)
 		    {
-		      if (cur == names->count)
-			{
-			  struct globnames *newnames;
-			  size_t count = names->count * 2;
-			  size_t nameoff = offsetof (struct globnames, name);
-			  size_t size = FLEXSIZEOF (struct globnames, name,
-						    count * sizeof (char *));
-			  if ((SIZE_MAX - nameoff) / 2 / sizeof (char *)
-			      < names->count)
-			    goto memory_error;
-			  if (glob_use_alloca (alloca_used, size))
-			    newnames = names_alloca
-			      = alloca_account (size, alloca_used);
-			  else if ((newnames = malloc (size))
-				   == NULL)
-			    goto memory_error;
-			  newnames->count = count;
-			  newnames->next = names;
-			  names = newnames;
-			  cur = 0;
-			}
-		      names->name[cur] = strdup (d.name);
-		      if (names->name[cur] == NULL)
+		      struct globnames *newnames;
+		      size_t count = names->count * 2;
+		      size_t nameoff = offsetof (struct globnames, name);
+		      size_t size = FLEXSIZEOF (struct globnames, name,
+						count * sizeof (char *));
+		      if ((SIZE_MAX - nameoff) / 2 / sizeof (char *)
+			  < names->count)
 			goto memory_error;
-		      ++cur;
-		      ++nfound;
-		      if (SIZE_MAX - pglob->gl_offs <= nfound)
+		      if (glob_use_alloca (alloca_used, size))
+			newnames = names_alloca
+			  = alloca_account (size, alloca_used);
+		      else if ((newnames = malloc (size))
+		 	       == NULL)
 			goto memory_error;
+		      newnames->count = count;
+		      newnames->next = names;
+		      names = newnames;
+		      cur = 0;
 		    }
+		  names->name[cur] = strdup (d.name);
+		  if (names->name[cur] == NULL)
+		    goto memory_error;
+		  ++cur;
+		  ++nfound;
+		  if (SIZE_MAX - pglob->gl_offs <= nfound)
+		    goto memory_error;
 		}
 	    }
 	}
diff --git a/posix/tst-glob_symlinks.c b/posix/tst-glob_symlinks.c
new file mode 100644
index 0000000..3aeac89
--- /dev/null
+++ b/posix/tst-glob_symlinks.c
@@ -0,0 +1,132 @@
+/* Test glob danglin symlink match (BZ #866).
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <unistd.h>
+#include <limits.h>
+#include <stddef.h>
+#include <glob.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+#include <support/check.h>
+#include <support/temp_file.h>
+
+static void do_prepare (int argc, char *argv[]);
+#define PREPARE do_prepare
+static int do_test (void);
+#include <support/test-driver.c>
+
+static void
+create_link (const char *base, const char *fname, char *linkname,
+	     size_t linknamesize)
+{
+  int ntries = 0;
+  while (1)
+    {
+      snprintf (linkname, linknamesize, "%s/%s%02d", test_dir, base,
+		ntries);
+      if (symlink (fname, linkname) == 0)
+	break;
+      if (errno != EEXIST)
+	FAIL_EXIT1 ("symlink failed: %m");
+      if (ntries++ == 10)
+	FAIL_EXIT1 ("symlink failed with EEXIST too many times");
+    }
+  add_temp_file (linkname);
+}
+
+static char valid_link[PATH_MAX];
+static char dangling_link[PATH_MAX];
+static char dangling_dir[PATH_MAX];
+
+static void
+do_prepare (int argc, char *argv[])
+{
+  char *fname;
+
+  create_temp_file ("tst-glob_symlinks.", &fname);
+
+  /* Create a existing symlink.  */
+  create_link ("valid-symlink-tst-glob_symlinks", fname, valid_link,
+	       sizeof valid_link);
+
+  /* Create a dangling symlink to a file.  */
+  int fd = create_temp_file ("dangling-tst-glob_file", &fname);
+  TEST_VERIFY_EXIT (close (fd) == 0);
+  /* It throws an warning at process end due 'add_temp_file' trying to
+     unlink it again.  */
+  TEST_VERIFY_EXIT (unlink (fname) == 0);
+  create_link ("dangling-symlink-file-tst-glob", fname, dangling_link,
+	       sizeof dangling_link);
+
+  /* Create a dangling symlink to a directory.  */
+  char tmpdir[PATH_MAX];
+  snprintf (tmpdir, sizeof tmpdir, "%s/dangling-tst-glob_folder.XXXXXX",
+	    test_dir);
+  TEST_VERIFY_EXIT (mkdtemp (tmpdir) != NULL);
+  create_link ("dangling-symlink-dir-tst-glob", tmpdir, dangling_dir,
+	       sizeof dangling_dir);
+  TEST_VERIFY_EXIT (rmdir (tmpdir) == 0);
+}
+
+static int
+do_test (void)
+{
+  char buf[PATH_MAX];
+  glob_t gl;
+
+  TEST_VERIFY_EXIT (glob (valid_link, 0, NULL, &gl) == 0);
+  TEST_VERIFY_EXIT (gl.gl_pathc == 1);
+  TEST_VERIFY_EXIT (strcmp (gl.gl_pathv[0], valid_link) == 0);
+  globfree (&gl);
+
+  TEST_VERIFY_EXIT (glob (dangling_link, 0, NULL, &gl) == 0);
+  TEST_VERIFY_EXIT (gl.gl_pathc == 1);
+  TEST_VERIFY_EXIT (strcmp (gl.gl_pathv[0], dangling_link) == 0);
+  globfree (&gl);
+
+  TEST_VERIFY_EXIT (glob (dangling_dir, 0, NULL, &gl) == 0);
+  TEST_VERIFY_EXIT (gl.gl_pathc == 1);
+  TEST_VERIFY_EXIT (strcmp (gl.gl_pathv[0], dangling_dir) == 0);
+  globfree (&gl);
+
+  snprintf (buf, sizeof buf, "%s", dangling_link);
+  buf[strlen(buf) - 1] = '?';
+  TEST_VERIFY_EXIT (glob (buf, 0, NULL, &gl) == 0);
+  TEST_VERIFY_EXIT (gl.gl_pathc == 1);
+  TEST_VERIFY_EXIT (strcmp (gl.gl_pathv[0], dangling_link) == 0);
+  globfree (&gl);
+
+  /* glob should handle dangling symbol as normal file, so <file>? should
+     return an empty string.  */
+  snprintf (buf, sizeof buf, "%s?", dangling_link);
+  TEST_VERIFY_EXIT (glob (buf, 0, NULL, &gl) != 0);
+  globfree (&gl);
+
+  snprintf (buf, sizeof buf, "%s*", dangling_link);
+  TEST_VERIFY_EXIT (glob (buf, 0, NULL, &gl) == 0);
+  TEST_VERIFY_EXIT (gl.gl_pathc == 1);
+  TEST_VERIFY_EXIT (strcmp (gl.gl_pathv[0], dangling_link) == 0);
+  globfree (&gl);
+
+  return 0;
+}
-- 
2.7.4

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 4/9] Sync scratch_buffer with gnulib
  2017-09-05 20:25 [PATCH 0/9] posix: glob fixes and refactor Adhemerval Zanella
                   ` (6 preceding siblings ...)
  2017-09-05 20:25 ` [PATCH 8/9] posix: Use enum for __glob_pattern_type result Adhemerval Zanella
@ 2017-09-05 20:25 ` Adhemerval Zanella
  2017-09-18  6:09   ` Florian Weimer
  2017-09-05 20:25 ` [PATCH 5/9] posix: Fix getpwnam_r usage (BZ #1062) Adhemerval Zanella
  8 siblings, 1 reply; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-05 20:25 UTC (permalink / raw)
  To: libc-alpha; +Cc: Paul Eggert

This patch syncs the scratch_buffer grom gnulib commit 3866ef6 with
GLIBC code.

Checked on x86_64-linux-gnu and on a build using build-many-glibcs.py
for all major architectures.

	* include/scratch_buffer.h (scratch_buffer): Use a C99 align method
	instead of GCC extension.
	* malloc/scratch_buffer_grow.c [!_LIBC]: Include libc-config.h.
	* malloc/scratch_buffer_grow_preserve.c [!_LIBC]: Likewise.
	* malloc/scratch_buffer_set_array_size.c [!_LIBC]: Likewise.
---
 ChangeLog                              | 6 ++++++
 include/scratch_buffer.h               | 3 +--
 malloc/scratch_buffer_grow.c           | 6 +++++-
 malloc/scratch_buffer_grow_preserve.c  | 6 +++++-
 malloc/scratch_buffer_set_array_size.c | 6 +++++-
 5 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/include/scratch_buffer.h b/include/scratch_buffer.h
index dd17a4a..bb04662 100644
--- a/include/scratch_buffer.h
+++ b/include/scratch_buffer.h
@@ -66,8 +66,7 @@
 struct scratch_buffer {
   void *data;    /* Pointer to the beginning of the scratch area.  */
   size_t length; /* Allocated space at the data pointer, in bytes.  */
-  char __space[1024]
-    __attribute__ ((aligned (__alignof__ (max_align_t))));
+  max_align_t __space[(1023 + sizeof (max_align_t)) / sizeof (max_align_t)];
 };
 
 /* Initializes *BUFFER so that BUFFER->data points to BUFFER->__space
diff --git a/malloc/scratch_buffer_grow.c b/malloc/scratch_buffer_grow.c
index 22bae50..d2df028 100644
--- a/malloc/scratch_buffer_grow.c
+++ b/malloc/scratch_buffer_grow.c
@@ -16,6 +16,10 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#ifndef _LIBC
+# include <libc-config.h>
+#endif
+
 #include <scratch_buffer.h>
 #include <errno.h>
 
@@ -49,4 +53,4 @@ __libc_scratch_buffer_grow (struct scratch_buffer *buffer)
   buffer->length = new_length;
   return true;
 }
-libc_hidden_def (__libc_scratch_buffer_grow);
+libc_hidden_def (__libc_scratch_buffer_grow)
diff --git a/malloc/scratch_buffer_grow_preserve.c b/malloc/scratch_buffer_grow_preserve.c
index 18543ef..9268615 100644
--- a/malloc/scratch_buffer_grow_preserve.c
+++ b/malloc/scratch_buffer_grow_preserve.c
@@ -16,6 +16,10 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#ifndef _LIBC
+# include <libc-config.h>
+#endif
+
 #include <scratch_buffer.h>
 #include <errno.h>
 #include <string.h>
@@ -60,4 +64,4 @@ __libc_scratch_buffer_grow_preserve (struct scratch_buffer *buffer)
   buffer->length = new_length;
   return true;
 }
-libc_hidden_def (__libc_scratch_buffer_grow_preserve);
+libc_hidden_def (__libc_scratch_buffer_grow_preserve)
diff --git a/malloc/scratch_buffer_set_array_size.c b/malloc/scratch_buffer_set_array_size.c
index 8ab6d9d..6fcc115 100644
--- a/malloc/scratch_buffer_set_array_size.c
+++ b/malloc/scratch_buffer_set_array_size.c
@@ -16,6 +16,10 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#ifndef _LIBC
+# include <libc-config.h>
+#endif
+
 #include <scratch_buffer.h>
 #include <errno.h>
 #include <limits.h>
@@ -57,4 +61,4 @@ __libc_scratch_buffer_set_array_size (struct scratch_buffer *buffer,
   buffer->length = new_length;
   return true;
 }
-libc_hidden_def (__libc_scratch_buffer_set_array_size);
+libc_hidden_def (__libc_scratch_buffer_set_array_size)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 5/9] posix: Fix getpwnam_r usage (BZ #1062)
  2017-09-05 20:25 [PATCH 0/9] posix: glob fixes and refactor Adhemerval Zanella
                   ` (7 preceding siblings ...)
  2017-09-05 20:25 ` [PATCH 4/9] Sync scratch_buffer with gnulib Adhemerval Zanella
@ 2017-09-05 20:25 ` Adhemerval Zanella
  8 siblings, 0 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-05 20:25 UTC (permalink / raw)
  To: libc-alpha; +Cc: Paul Eggert

This patch fixes longstanding misuse of errno after getpwnam_r,
which returns an error number rather than setting errno.  This is
sync with gnulib commit 5db9301.

Checked on x86_64-linux-gnu and on a build using build-many-glibcs.py
for all major architectures.

	[BZ #1062]
	* posix/glob.c (glob): Port recent patches to platforms
	lacking getpwnam_r.
	(glob): Fix longstanding misuse of errno after getpwnam_r, which
	returns an error number rather than setting errno.
---
 ChangeLog    |   6 +++
 posix/glob.c | 164 +++++++++--------------------------------------------------
 2 files changed, 30 insertions(+), 140 deletions(-)

diff --git a/posix/glob.c b/posix/glob.c
index 15c6295..340cf08 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -15,10 +15,6 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#ifndef _LIBC
-# include <config.h>
-#endif
-
 #include <glob.h>
 
 #include <errno.h>
@@ -39,10 +35,6 @@
 #endif
 
 #include <errno.h>
-#ifndef __set_errno
-# define __set_errno(val) errno = (val)
-#endif
-
 #include <dirent.h>
 #include <stdlib.h>
 #include <string.h>
@@ -82,12 +74,8 @@
 
 #include <flexmember.h>
 #include <glob_internal.h>
+#include <scratch_buffer.h>
 
-#ifdef _SC_GETPW_R_SIZE_MAX
-# define GETPW_R_SIZE_MAX()	sysconf (_SC_GETPW_R_SIZE_MAX)
-#else
-# define GETPW_R_SIZE_MAX()	(-1)
-#endif
 #ifdef _SC_LOGIN_NAME_MAX
 # define GET_LOGIN_NAME_MAX()	sysconf (_SC_LOGIN_NAME_MAX)
 #else
@@ -649,97 +637,36 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      if (success)
 		{
 		  struct passwd *p;
-		  char *malloc_pwtmpbuf = NULL;
-		  char *pwtmpbuf;
+		  struct scratch_buffer pwtmpbuf;
+		  scratch_buffer_init (&pwtmpbuf);
 # if defined HAVE_GETPWNAM_R || defined _LIBC
-		  long int pwbuflenmax = GETPW_R_SIZE_MAX ();
-		  size_t pwbuflen = pwbuflenmax;
 		  struct passwd pwbuf;
-		  int save = errno;
 
-#  ifndef _LIBC
-		  if (! (0 < pwbuflenmax && pwbuflenmax <= SIZE_MAX))
-		    /* 'sysconf' does not support _SC_GETPW_R_SIZE_MAX.
-		       Try a moderate value.  */
-		    pwbuflen = 1024;
-#  endif
-		  if (glob_use_alloca (alloca_used, pwbuflen))
-		    pwtmpbuf = alloca_account (pwbuflen, alloca_used);
-		  else
+		  while (getpwnam_r (name, &pwbuf,
+				     pwtmpbuf.data, pwtmpbuf.length, &p)
+			 == ERANGE)
 		    {
-		      pwtmpbuf = malloc (pwbuflen);
-		      if (pwtmpbuf == NULL)
+		      if (!scratch_buffer_grow (&pwtmpbuf))
 			{
-			  if (__glibc_unlikely (malloc_name))
-			    free (name);
 			  retval = GLOB_NOSPACE;
 			  goto out;
 			}
-		      malloc_pwtmpbuf = pwtmpbuf;
-		    }
-
-		  while (getpwnam_r (name, &pwbuf, pwtmpbuf, pwbuflen, &p)
-			 != 0)
-		    {
-		      size_t newlen;
-		      bool v;
-		      if (errno != ERANGE)
-			{
-			  p = NULL;
-			  break;
-			}
-		      v = size_add_wrapv (pwbuflen, pwbuflen, &newlen);
-		      if (!v && malloc_pwtmpbuf == NULL
-			  && glob_use_alloca (alloca_used, newlen))
-			pwtmpbuf = extend_alloca_account (pwtmpbuf, pwbuflen,
-							  newlen, alloca_used);
-		      else
-			{
-			  char *newp = (v ? NULL
-					: realloc (malloc_pwtmpbuf, newlen));
-			  if (newp == NULL)
-			    {
-			      free (malloc_pwtmpbuf);
-			      if (__glibc_unlikely (malloc_name))
-				free (name);
-			      retval = GLOB_NOSPACE;
-			      goto out;
-			    }
-			  malloc_pwtmpbuf = pwtmpbuf = newp;
-			}
-		      pwbuflen = newlen;
-		      __set_errno (save);
 		    }
 # else
 		  p = getpwnam (name);
 # endif
-		  if (__glibc_unlikely (malloc_name))
-		    free (name);
 		  if (p != NULL)
 		    {
-		      if (malloc_pwtmpbuf == NULL)
-			home_dir = p->pw_dir;
-		      else
+		      home_dir = strdup (p->pw_dir);
+		      malloc_home_dir = 1;
+		      if (home_dir == NULL)
 			{
-			  size_t home_dir_len = strlen (p->pw_dir) + 1;
-			  if (glob_use_alloca (alloca_used, home_dir_len))
-			    home_dir = alloca_account (home_dir_len,
-						       alloca_used);
-			  else
-			    {
-			      home_dir = malloc (home_dir_len);
-			      if (home_dir == NULL)
-				{
-				  free (pwtmpbuf);
-				  retval = GLOB_NOSPACE;
-				  goto out;
-				}
-			      malloc_home_dir = 1;
-			    }
-			  memcpy (home_dir, p->pw_dir, home_dir_len);
+			  scratch_buffer_free (&pwtmpbuf);
+			  retval = GLOB_NOSPACE;
+			  goto out;
 			}
 		    }
-		  free (malloc_pwtmpbuf);
+		  scratch_buffer_free (&pwtmpbuf);
 		}
 	      else
 		{
@@ -876,61 +803,21 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  /* Look up specific user's home directory.  */
 	  {
 	    struct passwd *p;
-	    char *malloc_pwtmpbuf = NULL;
+	    struct scratch_buffer pwtmpbuf;
+	    scratch_buffer_init (&pwtmpbuf);
+
 #  if defined HAVE_GETPWNAM_R || defined _LIBC
-	    long int buflenmax = GETPW_R_SIZE_MAX ();
-	    size_t buflen = buflenmax;
-	    char *pwtmpbuf;
 	    struct passwd pwbuf;
-	    int save = errno;
-
-#   ifndef _LIBC
-	    if (! (0 <= buflenmax && buflenmax <= SIZE_MAX))
-	      /* Perhaps 'sysconf' does not support _SC_GETPW_R_SIZE_MAX.  Try a
-		 moderate value.  */
-	      buflen = 1024;
-#   endif
-	    if (glob_use_alloca (alloca_used, buflen))
-	      pwtmpbuf = alloca_account (buflen, alloca_used);
-	    else
+
+	    while (getpwnam_r (user_name, &pwbuf,
+			       pwtmpbuf.data, pwtmpbuf.length, &p)
+		   == ERANGE)
 	      {
-		pwtmpbuf = malloc (buflen);
-		if (pwtmpbuf == NULL)
+		if (!scratch_buffer_grow (&pwtmpbuf))
 		  {
-		  nomem_getpw:
-		    if (__glibc_unlikely (malloc_user_name))
-		      free (user_name);
 		    retval = GLOB_NOSPACE;
 		    goto out;
 		  }
-		malloc_pwtmpbuf = pwtmpbuf;
-	      }
-
-	    while (getpwnam_r (user_name, &pwbuf, pwtmpbuf, buflen, &p) != 0)
-	      {
-		size_t newlen;
-		bool v;
-		if (errno != ERANGE)
-		  {
-		    p = NULL;
-		    break;
-		  }
-		v = size_add_wrapv (buflen, buflen, &newlen);
-		if (!v && malloc_pwtmpbuf == NULL
-		    && glob_use_alloca (alloca_used, newlen))
-		  pwtmpbuf = extend_alloca_account (pwtmpbuf, buflen,
-						    newlen, alloca_used);
-		else
-		  {
-		    char *newp = v ? NULL : realloc (malloc_pwtmpbuf, newlen);
-		    if (newp == NULL)
-		      {
-			free (malloc_pwtmpbuf);
-			goto nomem_getpw;
-		      }
-		    malloc_pwtmpbuf = pwtmpbuf = newp;
-		  }
-		__set_errno (save);
 	      }
 #  else
 	    p = getpwnam (user_name);
@@ -957,7 +844,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		    dirname = malloc (home_len + rest_len + 1);
 		    if (dirname == NULL)
 		      {
-			free (malloc_pwtmpbuf);
+			scratch_buffer_free (&pwtmpbuf);
 			retval = GLOB_NOSPACE;
 			goto out;
 		      }
@@ -968,13 +855,9 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 
 		dirlen = home_len + rest_len;
 		dirname_modified = 1;
-
-		free (malloc_pwtmpbuf);
 	      }
 	    else
 	      {
-		free (malloc_pwtmpbuf);
-
 		if (flags & GLOB_TILDE_CHECK)
 		  {
 		    /* We have to regard it as an error if we cannot find the
@@ -983,6 +866,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		    goto out;
 		  }
 	      }
+	    scratch_buffer_free (&pwtmpbuf);
 	  }
 #endif /* !WINDOWS32 */
 	}
-- 
2.7.4

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 9/9] posix: Fix glob with GLOB_NOCHECK returning modified patterns (BZ#10246)
  2017-09-05 20:25 [PATCH 0/9] posix: glob fixes and refactor Adhemerval Zanella
@ 2017-09-05 20:25 ` Adhemerval Zanella
  2017-09-07 22:14   ` Paul Eggert
  2017-09-05 20:25 ` [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866] Adhemerval Zanella
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-05 20:25 UTC (permalink / raw)
  To: libc-alpha; +Cc: Paul Eggert

According to POSIX glob with GLOB_NOCHECK should return a list consisting
of only of the input pattern in case of no match.  However GLIBC does not
honor in case of '//<something'.  This is due internally this is handled
and special case and prefix_array (responsable to prepend the directory
name) does not know if the input already contains a slash or not since
either '/<something>' or '//<something>' will be handle in same way.

This patch fix it by using a empty directory name for the latter (since
prefix_array already adds a slash as default for each entry).

Checked on x86_64-linux-gnu and on a build using build-many-glibcs.py
for all major architectures.

	[BZ #10246]
	* posix/glob.c (glob): Handle pattern that do not match and
	start with '/' correctly.
	* posix/globtest.sh: New tests for NOCHECK.
---
 ChangeLog         |  7 ++++++-
 posix/glob.c      | 13 +++++++------
 posix/globtest.sh | 37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/posix/glob.c b/posix/glob.c
index 30a4143..25c5d24 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -272,6 +272,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
   size_t oldcount;
   int meta;
   int dirname_modified;
+  /* Indicate if the directory should be prepended on return values.  */
+  bool dirname_prefix = true;
   int malloc_dirname = 0;
   glob_t dirs;
   int retval = 0;
@@ -495,6 +497,10 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
       dirname = (char *) "/";
       dirlen = 1;
       ++filename;
+      /* prefix_array adds a separator for each result and DIRNAME is
+	 already '/'.  So we indicate later that we should not prepend
+	 anything for this specific case.  */
+      dirname_prefix = false;
     }
   else
     {
@@ -1086,7 +1092,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
       if (dirlen > 0)
 	{
 	  /* Stick the directory on the front of each name.  */
-	  if (prefix_array (dirname,
+	  if (prefix_array (dirname_prefix ? dirname : "",
 			    &pglob->gl_pathv[old_pathc + pglob->gl_offs],
 			    pglob->gl_pathc - old_pathc))
 	    {
@@ -1167,11 +1173,6 @@ prefix_array (const char *dirname, char **array, size_t n)
   size_t dirlen = strlen (dirname);
   char dirsep_char = '/';
 
-  if (dirlen == 1 && dirname[0] == '/')
-    /* DIRNAME is just "/", so normal prepending would get us "//foo".
-       We want "/foo" instead, so don't prepend any chars from DIRNAME.  */
-    dirlen = 0;
-
 #if defined __MSDOS__ || defined WINDOWS32
   if (dirlen > 1)
     {
diff --git a/posix/globtest.sh b/posix/globtest.sh
index 73f7ae3..92a8e37 100755
--- a/posix/globtest.sh
+++ b/posix/globtest.sh
@@ -242,6 +242,43 @@ if test $failed -ne 0; then
   result=1
 fi
 
+# Test NOCHECK for specific cases where the pattern used starts
+# with '/' (BZ#10246).
+failed=0
+${test_program_prefix} \
+${common_objpfx}posix/globtest -c "$testdir" "/%" |
+sort > $testout
+cat <<"EOF" | $CMP - $testout >> $logfile || failed=1
+`/%'
+EOF
+if test $failed -ne 0; then
+  echo "No check test failed" >> $logfile
+  result=1
+fi
+
+${test_program_prefix} \
+${common_objpfx}posix/globtest -c "$testdir" "//%" |
+sort > $testout
+cat <<"EOF" | $CMP - $testout >> $logfile || failed=1
+`//%'
+EOF
+if test $failed -ne 0; then
+  echo "No check test failed" >> $logfile
+  result=1
+fi
+
+${test_program_prefix} \
+${common_objpfx}posix/globtest -c "$testdir" "///%" |
+sort > $testout
+cat <<"EOF" | $CMP - $testout >> $logfile || failed=1
+`///%'
+EOF
+if test $failed -ne 0; then
+  echo "No check test failed" >> $logfile
+  result=1
+fi
+
+
 # Test NOMAGIC without magic characters
 failed=0
 ${test_program_prefix} \
-- 
2.7.4

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 1/9] posix: Sync glob with gnulib [BZ #1062]
  2017-09-05 20:25 [PATCH 0/9] posix: glob fixes and refactor Adhemerval Zanella
                   ` (2 preceding siblings ...)
  2017-09-05 20:25 ` [PATCH 6/9] posix: fix glob bugs with long login names Adhemerval Zanella
@ 2017-09-05 20:25 ` Adhemerval Zanella
  2017-09-06  2:01   ` Paul Eggert
  2017-09-12 14:20   ` Andreas Schwab
  2017-09-05 20:25 ` [PATCH 7/9] posix: Consolidate glob implementation Adhemerval Zanella
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-05 20:25 UTC (permalink / raw)
  To: libc-alpha; +Cc: Paul Eggert

This patch syncs posix/glob.c implementation with gnulib version
b5ec983 (glob: simplify symlink detection).  The only difference
to gnulib code is

  * DT_UNKNOWN, DT_DIR, and DT_LNK definition in the case there
    were not already defined.  Gnulib code which uses
    HAVE_STRUCT_DIRENT_D_TYPE will redefine them wrongly because
    GLIBC does not define HAVE_STRUCT_DIRENT_D_TYPE.  Instead
    the patch check for each definition instead.

Also, the patch requires additional globfree and globfree64 files
for compatibility version on some architectures.  Also the code
simplification leads to not macro simplification (not need for
NO_GLOB_PATTERN_P anymore).

Checked on x86_64-linux-gnu and on a build using build-many-glibcs.py
for all major architectures.

	[BZ #1062]
	* posix/Makefile (routines): Add globfree, globfree64, and
	glob_pattern_p.
	* posix/flexmember.h: New file.
	* posix/glob_internal.h: Likewise.
	* posix/glob_pattern_p.c: Likewise.
	* posix/globfree.c: Likewise.
	* posix/globfree64.c: Likewise.
	* sysdeps/gnu/globfree64.c: Likewise.
	* sysdeps/unix/sysv/linux/alpha/globfree.c: Likewise.
	* sysdeps/unix/sysv/linux/mips/mips64/n64/globfree64.c: Likewise.
	* sysdeps/unix/sysv/linux/oldglob.c: Likewise.
	* sysdeps/unix/sysv/linux/wordsize-64/globfree64.c: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/x32/globfree.c: Likewise.
	* sysdeps/wordsize-64/globfree.c: Likewise.
	* sysdeps/wordsize-64/globfree64.c: Likewise.
	* posix/glob.c (HAVE_CONFIG_H): Use !_LIBC instead.
	[NDEBUG): Remove comments.
	(GLOB_ONLY_P, _AMIGA, VMS): Remove define.
	(dirent_type): New type.  Use uint_fast8_t not
	uint8_t, as C99 does not require uint8_t.
	(DT_UNKNOWN, DT_DIR, DT_LNK): New macros.
	(struct readdir_result): Use dirent_type.  Do not define skip_entry
	unless it is needed; this saves a byte on platforms lacking d_ino.
	(readdir_result_type, readdir_result_skip_entry):
	New functions, replacing ...
	(readdir_result_might_be_symlink, readdir_result_might_be_dir):
	 these functions, which were removed.  This makes the callers
	easier to read.  All callers changed.
	(D_INO_TO_RESULT): Now empty if there is no d_ino.
	(size_add_wrapv, glob_use_alloca): New static functions.
	(glob, glob_in_dir): Check for size_t overflow in several places,
	and fix some size_t checks that were not quite right.
	Remove old code using SHELL since Bash no longer
	uses this.
	(glob, prefix_array): Separate MS code better.
	(glob_in_dir): Remove old Amiga and VMS code.
	(globfree, __glob_pattern_type, __glob_pattern_p): Move to
	separate files.
	(glob_in_dir): Do not rely on undefined behavior in accessing
	struct members beyond their bounds.  Use a flexible array member
	instead
	(link_stat): Rename from link_exists2_p and return -1/0 instead of
	0/1.  Caller changed.
	(glob): Fix memory leaks.
	* posix/glob64 (globfree64): Move to separate file.
	* sysdeps/gnu/glob64.c (NO_GLOB_PATTERN_P): Remove define.
	(globfree64): Remove hidden alias.
	* sysdeps/unix/sysv/linux/Makefile (sysdeps_routines): Add
	oldglob.
	* sysdeps/unix/sysv/linux/alpha/glob.c (__new_globfree): Move to
	separate file.
	* sysdeps/unix/sysv/linux/i386/glob64.c (NO_GLOB_PATTERN_P): Remove
	define.
	Move compat code to separate file.
	* sysdeps/wordsize-64/glob.c (globfree): Move definitions to
	separate file.
---
 ChangeLog                                          |  60 ++
 posix/Makefile                                     |   2 +-
 posix/flexmember.h                                 |  45 ++
 posix/glob.c                                       | 775 ++++++++++-----------
 posix/glob64.c                                     |   6 -
 posix/glob_internal.h                              |  57 ++
 posix/glob_pattern_p.c                             |  33 +
 posix/globfree.c                                   |  41 ++
 posix/globfree64.c                                 |  31 +
 sysdeps/gnu/glob64.c                               |   3 -
 sysdeps/gnu/globfree64.c                           |  10 +
 sysdeps/unix/sysv/linux/Makefile                   |   2 +-
 sysdeps/unix/sysv/linux/alpha/glob.c               |   4 -
 sysdeps/unix/sysv/linux/alpha/globfree.c           |  37 +
 sysdeps/unix/sysv/linux/i386/glob64.c              |  39 +-
 .../unix/sysv/linux/mips/mips64/n64/globfree64.c   |   1 +
 sysdeps/unix/sysv/linux/oldglob.c                  |  42 ++
 sysdeps/unix/sysv/linux/wordsize-64/globfree64.c   |   2 +
 sysdeps/unix/sysv/linux/x86_64/x32/globfree.c      |   1 +
 sysdeps/wordsize-64/glob.c                         |   2 -
 sysdeps/wordsize-64/globfree.c                     |   5 +
 sysdeps/wordsize-64/globfree64.c                   |   1 +
 22 files changed, 740 insertions(+), 459 deletions(-)
 create mode 100644 posix/flexmember.h
 create mode 100644 posix/glob_internal.h
 create mode 100644 posix/glob_pattern_p.c
 create mode 100644 posix/globfree.c
 create mode 100644 posix/globfree64.c
 create mode 100644 sysdeps/gnu/globfree64.c
 create mode 100644 sysdeps/unix/sysv/linux/alpha/globfree.c
 create mode 100644 sysdeps/unix/sysv/linux/mips/mips64/n64/globfree64.c
 create mode 100644 sysdeps/unix/sysv/linux/oldglob.c
 create mode 100644 sysdeps/unix/sysv/linux/wordsize-64/globfree64.c
 create mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/globfree.c
 create mode 100644 sysdeps/wordsize-64/globfree.c
 create mode 100644 sysdeps/wordsize-64/globfree64.c

diff --git a/posix/Makefile b/posix/Makefile
index 9b534f0..0ff8f5c 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -45,7 +45,7 @@ routines :=								      \
 	getpgid setpgid getpgrp bsd-getpgrp setpgrp getsid setsid	      \
 	getresuid getresgid setresuid setresgid				      \
 	pathconf sysconf fpathconf					      \
-	glob glob64 fnmatch regex					      \
+	glob glob64 globfree globfree64 glob_pattern_p fnmatch regex	      \
 	confstr								      \
 	getopt getopt1 							      \
 	sched_setp sched_getp sched_sets sched_gets sched_yield sched_primax  \
diff --git a/posix/flexmember.h b/posix/flexmember.h
new file mode 100644
index 0000000..107c1f0
--- /dev/null
+++ b/posix/flexmember.h
@@ -0,0 +1,45 @@
+/* Sizes of structs with flexible array members.
+
+   Copyright 2016-2017 Free Software Foundation, Inc.
+
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.
+
+   Written by Paul Eggert.  */
+
+#include <stddef.h>
+
+/* Nonzero multiple of alignment of TYPE, suitable for FLEXSIZEOF below.
+   On older platforms without _Alignof, use a pessimistic bound that is
+   safe in practice even if FLEXIBLE_ARRAY_MEMBER is 1.
+   On newer platforms, use _Alignof to get a tighter bound.  */
+
+#if !defined __STDC_VERSION__ || __STDC_VERSION__ < 201112
+# define FLEXALIGNOF(type) (sizeof (type) & ~ (sizeof (type) - 1))
+#else
+# define FLEXALIGNOF(type) _Alignof (type)
+#endif
+
+/* Upper bound on the size of a struct of type TYPE with a flexible
+   array member named MEMBER that is followed by N bytes of other data.
+   This is not simply sizeof (TYPE) + N, since it may require
+   alignment on unusually picky C11 platforms, and
+   FLEXIBLE_ARRAY_MEMBER may be 1 on pre-C11 platforms.
+   Yield a value less than N if and only if arithmetic overflow occurs.  */
+
+#define FLEXSIZEOF(type, member, n) \
+   ((offsetof (type, member) + FLEXALIGNOF (type) - 1 + (n)) \
+    & ~ (FLEXALIGNOF (type) - 1))
diff --git a/posix/glob.c b/posix/glob.c
index c653809..e19aa6f 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -15,7 +15,7 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#ifdef	HAVE_CONFIG_H
+#ifndef _LIBC
 # include <config.h>
 #endif
 
@@ -27,29 +27,15 @@
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
-
-/* Outcomment the following line for production quality code.  */
-/* #define NDEBUG 1 */
 #include <assert.h>
+#include <unistd.h>
 
-#include <stdio.h>		/* Needed on stupid SunOS for assert.  */
-
-#if !defined _LIBC || !defined GLOB_ONLY_P
-#if defined HAVE_UNISTD_H || defined _LIBC
-# include <unistd.h>
-# ifndef POSIX
-#  ifdef _POSIX_VERSION
-#   define POSIX
-#  endif
-# endif
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+# define WINDOWS32
 #endif
 
-#include <pwd.h>
-
-#if defined HAVE_STDINT_H || defined _LIBC
-# include <stdint.h>
-#elif !defined UINTPTR_MAX
-# define UINTPTR_MAX (~((size_t) 0))
+#ifndef WINDOWS32
+# include <pwd.h>
 #endif
 
 #include <errno.h>
@@ -57,24 +43,7 @@
 # define __set_errno(val) errno = (val)
 #endif
 
-#if defined HAVE_DIRENT_H || defined __GNU_LIBRARY__
-# include <dirent.h>
-#else
-# define dirent direct
-# ifdef HAVE_SYS_NDIR_H
-#  include <sys/ndir.h>
-# endif
-# ifdef HAVE_SYS_DIR_H
-#  include <sys/dir.h>
-# endif
-# ifdef HAVE_NDIR_H
-#  include <ndir.h>
-# endif
-# ifdef HAVE_VMSDIR_H
-#  include "vmsdir.h"
-# endif /* HAVE_VMSDIR_H */
-#endif
-
+#include <dirent.h>
 #include <stdlib.h>
 #include <string.h>
 #include <alloca.h>
@@ -87,27 +56,29 @@
 # define opendir(name) __opendir (name)
 # define readdir(str) __readdir64 (str)
 # define getpwnam_r(name, bufp, buf, len, res) \
-   __getpwnam_r (name, bufp, buf, len, res)
+    __getpwnam_r (name, bufp, buf, len, res)
 # ifndef __stat64
 #  define __stat64(fname, buf) __xstat64 (_STAT_VER, fname, buf)
 # endif
 # define struct_stat64		struct stat64
+# define FLEXIBLE_ARRAY_MEMBER
 #else /* !_LIBC */
-# include "getlogin_r.h"
-# include "mempcpy.h"
-# include "stat-macros.h"
-# include "strdup.h"
-# define __stat64(fname, buf)	stat (fname, buf)
-# define struct_stat64		struct stat
-# define __stat(fname, buf)	stat (fname, buf)
-# define __alloca		alloca
-# define __readdir		readdir
-# define __readdir64		readdir64
-# define __glob_pattern_p	glob_pattern_p
+# define __getlogin_r(buf, len) getlogin_r (buf, len)
+# define __stat64(fname, buf)   stat (fname, buf)
+# define __fxstatat64(_, d, f, st, flag) fstatat (d, f, st, flag)
+# define struct_stat64          struct stat
+# ifndef __MVS__
+#  define __alloca              alloca
+# endif
+# define __readdir              readdir
+# define COMPILE_GLOB64
 #endif /* _LIBC */
 
 #include <fnmatch.h>
 
+#include <flexmember.h>
+#include <glob_internal.h>
+
 #ifdef _SC_GETPW_R_SIZE_MAX
 # define GETPW_R_SIZE_MAX()	sysconf (_SC_GETPW_R_SIZE_MAX)
 #else
@@ -121,61 +92,60 @@
 \f
 static const char *next_brace_sub (const char *begin, int flags) __THROWNL;
 
+typedef uint_fast8_t dirent_type;
+
+/* Any distinct values will do here.
+   Undef any existing macros out of the way.  */
+#ifndef DT_UNKNOWN
+# define DT_UNKNOWN 0
+#endif
+#ifndef DT_DIR
+# define DT_DIR 1
+#endif
+#ifndef DT_LNK
+# define DT_LNK 2
+#endif
+
 /* A representation of a directory entry which does not depend on the
    layout of struct dirent, or the size of ino_t.  */
 struct readdir_result
 {
   const char *name;
-# if defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE
-  uint8_t type;
-# endif
+#if defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE
+  dirent_type type;
+#endif
+#if defined _LIBC || defined D_INO_IN_DIRENT
   bool skip_entry;
+#endif
 };
 
-# if defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE
-/* Initializer based on the d_type member of struct dirent.  */
-#  define D_TYPE_TO_RESULT(source) (source)->d_type,
-
-/* True if the directory entry D might be a symbolic link.  */
-static bool
-readdir_result_might_be_symlink (struct readdir_result d)
-{
-  return d.type == DT_UNKNOWN || d.type == DT_LNK;
-}
-
-/* True if the directory entry D might be a directory.  */
-static bool
-readdir_result_might_be_dir (struct readdir_result d)
-{
-  return d.type == DT_DIR || readdir_result_might_be_symlink (d);
-}
-# else /* defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE */
-#  define D_TYPE_TO_RESULT(source)
-
-/* If we do not have type information, symbolic links and directories
-   are always a possibility.  */
-
-static bool
-readdir_result_might_be_symlink (struct readdir_result d)
+/* Initialize and return type member of struct readdir_result.  */
+static dirent_type
+readdir_result_type (struct readdir_result d)
 {
-  return true;
+#if defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE
+# define D_TYPE_TO_RESULT(source) (source)->d_type,
+  return d.type;
+#else
+# define D_TYPE_TO_RESULT(source)
+  return DT_UNKNOWN;
+#endif
 }
 
+/* Initialize and return skip_entry member of struct readdir_result.  */
 static bool
-readdir_result_might_be_dir (struct readdir_result d)
+readdir_result_skip_entry (struct readdir_result d)
 {
-  return true;
-}
-
-# endif /* defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE */
-
-# if (defined POSIX || defined WINDOWS32) && !defined __GNU_LIBRARY__
 /* Initializer for skip_entry.  POSIX does not require that the d_ino
    field be present, and some systems do not provide it. */
-#  define D_INO_TO_RESULT(source) false,
-# else
-#  define D_INO_TO_RESULT(source) (source)->d_ino == 0,
-# endif
+#if defined _LIBC || defined D_INO_IN_DIRENT
+# define D_INO_TO_RESULT(source) (source)->d_ino == 0,
+  return d.skip_entry;
+#else
+# define D_INO_TO_RESULT(source)
+  return false;
+#endif
+}
 
 /* Construct an initializer for a struct readdir_result object from a
    struct dirent *.  No copy of the name is made.  */
@@ -186,8 +156,6 @@ readdir_result_might_be_dir (struct readdir_result d)
     D_INO_TO_RESULT (source)		   \
   }
 
-#endif /* !defined _LIBC || !defined GLOB_ONLY_P */
-
 /* Call gl_readdir on STREAM.  This macro can be overridden to reduce
    type safety if an old interface version needs to be supported.  */
 #ifndef GL_READDIR
@@ -225,10 +193,48 @@ convert_dirent64 (const struct dirent64 *source)
 }
 #endif
 
+#ifndef _LIBC
+/* The results of opendir() in this file are not used with dirfd and fchdir,
+   and we do not leak fds to any single-threaded code that could use stdio,
+   therefore save some unnecessary recursion in fchdir.c and opendir_safer.c.
+   FIXME - if the kernel ever adds support for multi-thread safety for
+   avoiding standard fds, then we should use opendir_safer.  */
+# ifdef GNULIB_defined_opendir
+#  undef opendir
+# endif
+# ifdef GNULIB_defined_closedir
+#  undef closedir
+# endif
+
+/* Just use malloc.  */
+# define __libc_use_alloca(n) false
+# define alloca_account(len, avar) ((void) (len), (void) (avar), (void *) 0)
+# define extend_alloca_account(buf, len, newlen, avar) \
+    ((void) (buf), (void) (len), (void) (newlen), (void) (avar), (void *) 0)
+#endif
+
+/* Set *R = A + B.  Return true if the answer is mathematically
+   incorrect due to overflow; in this case, *R is the low order
+   bits of the correct answer.  */
 
-#ifndef attribute_hidden
-# define attribute_hidden
+static bool
+size_add_wrapv (size_t a, size_t b, size_t *r)
+{
+#if 5 <= __GNUC__ && !defined __ICC
+  return __builtin_add_overflow (a, b, r);
+#else
+  *r = a + b;
+  return *r < a;
 #endif
+}
+
+static bool
+glob_use_alloca (size_t alloca_used, size_t len)
+{
+  size_t size;
+  return (!size_add_wrapv (alloca_used, len, &size)
+          && __libc_use_alloca (size));
+}
 
 static int glob_in_dir (const char *pattern, const char *directory,
 			int flags, int (*errfunc) (const char *, int),
@@ -236,7 +242,6 @@ static int glob_in_dir (const char *pattern, const char *directory,
 extern int __glob_pattern_type (const char *pattern, int quote)
     attribute_hidden;
 
-#if !defined _LIBC || !defined GLOB_ONLY_P
 static int prefix_array (const char *prefix, char **array, size_t n) __THROWNL;
 static int collated_compare (const void *, const void *) __THROWNL;
 
@@ -265,16 +270,15 @@ next_brace_sub (const char *cp, int flags)
   return *cp != '\0' ? cp : NULL;
 }
 
-#endif /* !defined _LIBC || !defined GLOB_ONLY_P */
 
 /* Do glob searching for PATTERN, placing results in PGLOB.
    The bits defined above may be set in FLAGS.
    If a directory cannot be opened or read and ERRFUNC is not nil,
    it is called with the pathname that caused the error, and the
-   `errno' value from the failing call; if it returns non-zero
-   `glob' returns GLOB_ABORTED; if it returns zero, the error is ignored.
+   'errno' value from the failing call; if it returns non-zero
+   'glob' returns GLOB_ABORTED; if it returns zero, the error is ignored.
    If memory cannot be allocated for PGLOB, GLOB_NOSPACE is returned.
-   Otherwise, `glob' returns zero.  */
+   Otherwise, 'glob' returns zero.  */
 int
 #ifdef GLOB_ATTRIBUTE
 GLOB_ATTRIBUTE
@@ -292,9 +296,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
   int malloc_dirname = 0;
   glob_t dirs;
   int retval = 0;
-#ifdef _LIBC
   size_t alloca_used = 0;
-#endif
 
   if (pattern == NULL || pglob == NULL || (flags & ~__GLOB_FLAGS) != 0)
     {
@@ -308,7 +310,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
     flags |= GLOB_ONLYDIR;
 
   if (!(flags & GLOB_DOOFFS))
-    /* Have to do this so `globfree' knows where to start freeing.  It
+    /* Have to do this so 'globfree' knows where to start freeing.  It
        also makes all the code that uses gl_offs simpler. */
     pglob->gl_offs = 0;
 
@@ -372,14 +374,12 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  size_t rest_len;
 	  char *onealt;
 	  size_t pattern_len = strlen (pattern) - 1;
-#ifdef _LIBC
-	  int alloca_onealt = __libc_use_alloca (alloca_used + pattern_len);
+	  int alloca_onealt = glob_use_alloca (alloca_used, pattern_len);
 	  if (alloca_onealt)
 	    onealt = alloca_account (pattern_len, alloca_used);
 	  else
-#endif
 	    {
-	      onealt = (char *) malloc (pattern_len);
+	      onealt = malloc (pattern_len);
 	      if (onealt == NULL)
 		return GLOB_NOSPACE;
 	    }
@@ -392,11 +392,9 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  next = next_brace_sub (begin + 1, flags);
 	  if (next == NULL)
 	    {
-	      /* It is an illegal expression.  */
+	      /* It is an invalid expression.  */
 	    illegal_brace:
-#ifdef _LIBC
 	      if (__glibc_unlikely (!alloca_onealt))
-#endif
 		free (onealt);
 	      flags &= ~GLOB_BRACE;
 	      goto no_brace;
@@ -437,9 +435,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      /* If we got an error, return it.  */
 	      if (result && result != GLOB_NOMATCH)
 		{
-#ifdef _LIBC
 		  if (__glibc_unlikely (!alloca_onealt))
-#endif
 		    free (onealt);
 		  if (!(flags & GLOB_APPEND))
 		    {
@@ -458,9 +454,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      assert (next != NULL);
 	    }
 
-#ifdef _LIBC
 	  if (__glibc_unlikely (!alloca_onealt))
-#endif
 	    free (onealt);
 
 	  if (pglob->gl_pathc != firstc)
@@ -476,14 +470,16 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 
   /* Find the filename.  */
   filename = strrchr (pattern, '/');
+
 #if defined __MSDOS__ || defined WINDOWS32
-  /* The case of "d:pattern".  Since `:' is not allowed in
+  /* The case of "d:pattern".  Since ':' is not allowed in
      file names, we can safely assume that wherever it
      happens in pattern, it signals the filename part.  This
      is so we could some day support patterns like "[a-z]:foo".  */
   if (filename == NULL)
     filename = strchr (pattern, ':');
 #endif /* __MSDOS__ || WINDOWS32 */
+
   dirname_modified = 0;
   if (filename == NULL)
     {
@@ -508,11 +504,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	    }
 
 	  filename = pattern;
-#ifdef _AMIGA
-	  dirname = (char *) "";
-#else
 	  dirname = (char *) ".";
-#endif
 	  dirlen = 0;
 	}
     }
@@ -536,22 +528,21 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  char *drive_spec;
 
 	  ++dirlen;
-	  drive_spec = (char *) __alloca (dirlen + 1);
+	  drive_spec = __alloca (dirlen + 1);
 	  *((char *) mempcpy (drive_spec, pattern, dirlen)) = '\0';
 	  /* For now, disallow wildcards in the drive spec, to
 	     prevent infinite recursion in glob.  */
 	  if (__glob_pattern_p (drive_spec, !(flags & GLOB_NOESCAPE)))
 	    return GLOB_NOMATCH;
-	  /* If this is "d:pattern", we need to copy `:' to DIRNAME
+	  /* If this is "d:pattern", we need to copy ':' to DIRNAME
 	     as well.  If it's "d:/pattern", don't remove the slash
 	     from "d:/", since "d:" and "d:/" are not the same.*/
 	}
 #endif
-#ifdef _LIBC
-      if (__libc_use_alloca (alloca_used + dirlen + 1))
+
+      if (glob_use_alloca (alloca_used, dirlen + 1))
 	newp = alloca_account (dirlen + 1, alloca_used);
       else
-#endif
 	{
 	  newp = malloc (dirlen + 1);
 	  if (newp == NULL)
@@ -562,14 +553,17 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
       dirname = newp;
       ++filename;
 
-      if (filename[0] == '\0'
 #if defined __MSDOS__ || defined WINDOWS32
-	  && dirname[dirlen - 1] != ':'
-	  && (dirlen < 3 || dirname[dirlen - 2] != ':'
-	      || dirname[dirlen - 1] != '/')
+      bool drive_root = (dirlen > 1
+                         && (dirname[dirlen - 1] == ':'
+                             || (dirlen > 2 && dirname[dirlen - 2] == ':'
+                                 && dirname[dirlen - 1] == '/')));
+#else
+      bool drive_root = false;
 #endif
-	  && dirlen > 1)
-	/* "pattern/".  Expand "pattern", appending slashes.  */
+
+      if (filename[0] == '\0' && dirlen > 1 && !drive_root)
+        /* "pattern/".  Expand "pattern", appending slashes.  */
 	{
 	  int orig_flags = flags;
 	  if (!(flags & GLOB_NOESCAPE) && dirname[dirlen - 1] == '\\')
@@ -602,7 +596,6 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	}
     }
 
-#ifndef VMS
   if ((flags & (GLOB_TILDE|GLOB_TILDE_CHECK)) && dirname[0] == '~')
     {
       if (dirname[1] == '\0' || dirname[1] == '/'
@@ -612,100 +605,127 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  /* Look up home directory.  */
 	  char *home_dir = getenv ("HOME");
 	  int malloc_home_dir = 0;
-# ifdef _AMIGA
-	  if (home_dir == NULL || home_dir[0] == '\0')
-	    home_dir = "SYS:";
-# else
-#  ifdef WINDOWS32
-	  if (home_dir == NULL || home_dir[0] == '\0')
-	    home_dir = "c:/users/default"; /* poor default */
-#  else
 	  if (home_dir == NULL || home_dir[0] == '\0')
 	    {
+#ifdef WINDOWS32
+	      /* Windows NT defines HOMEDRIVE and HOMEPATH.  But give
+		 preference to HOME, because the user can change HOME.  */
+	      const char *home_drive = getenv ("HOMEDRIVE");
+	      const char *home_path = getenv ("HOMEPATH");
+
+	      if (home_drive != NULL && home_path != NULL)
+		{
+		  size_t home_drive_len = strlen (home_drive);
+		  size_t home_path_len = strlen (home_path);
+		  char *mem = alloca (home_drive_len + home_path_len + 1);
+
+		  memcpy (mem, home_drive, home_drive_len);
+		  memcpy (mem + home_drive_len, home_path, home_path_len + 1);
+		  home_dir = mem;
+		}
+	      else
+		home_dir = "c:/users/default"; /* poor default */
+#else
 	      int success;
 	      char *name;
+	      int malloc_name = 0;
 	      size_t buflen = GET_LOGIN_NAME_MAX () + 1;
 
 	      if (buflen == 0)
-		/* `sysconf' does not support _SC_LOGIN_NAME_MAX.  Try
+		/* 'sysconf' does not support _SC_LOGIN_NAME_MAX.  Try
 		   a moderate value.  */
 		buflen = 20;
-	      name = alloca_account (buflen, alloca_used);
+	      if (glob_use_alloca (alloca_used, buflen))
+		name = alloca_account (buflen, alloca_used);
+	      else
+		{
+		  name = malloc (buflen);
+		  if (name == NULL)
+		    {
+		      retval = GLOB_NOSPACE;
+		      goto out;
+		    }
+		  malloc_name = 1;
+		}
 
 	      success = __getlogin_r (name, buflen) == 0;
 	      if (success)
 		{
 		  struct passwd *p;
-#   if defined HAVE_GETPWNAM_R || defined _LIBC
-		  long int pwbuflen = GETPW_R_SIZE_MAX ();
+		  char *malloc_pwtmpbuf = NULL;
 		  char *pwtmpbuf;
+# if defined HAVE_GETPWNAM_R || defined _LIBC
+		  long int pwbuflenmax = GETPW_R_SIZE_MAX ();
+		  size_t pwbuflen = pwbuflenmax;
 		  struct passwd pwbuf;
-		  int malloc_pwtmpbuf = 0;
 		  int save = errno;
 
-#    ifndef _LIBC
-		  if (pwbuflen == -1)
-		    /* `sysconf' does not support _SC_GETPW_R_SIZE_MAX.
+#  ifndef _LIBC
+		  if (! (0 < pwbuflenmax && pwbuflenmax <= SIZE_MAX))
+		    /* 'sysconf' does not support _SC_GETPW_R_SIZE_MAX.
 		       Try a moderate value.  */
 		    pwbuflen = 1024;
-#    endif
-		  if (__libc_use_alloca (alloca_used + pwbuflen))
+#  endif
+		  if (glob_use_alloca (alloca_used, pwbuflen))
 		    pwtmpbuf = alloca_account (pwbuflen, alloca_used);
 		  else
 		    {
 		      pwtmpbuf = malloc (pwbuflen);
 		      if (pwtmpbuf == NULL)
 			{
+			  if (__glibc_unlikely (malloc_name))
+			    free (name);
 			  retval = GLOB_NOSPACE;
 			  goto out;
 			}
-		      malloc_pwtmpbuf = 1;
+		      malloc_pwtmpbuf = pwtmpbuf;
 		    }
 
 		  while (getpwnam_r (name, &pwbuf, pwtmpbuf, pwbuflen, &p)
 			 != 0)
 		    {
+		      size_t newlen;
+		      bool v;
 		      if (errno != ERANGE)
 			{
 			  p = NULL;
 			  break;
 			}
-
-		      if (!malloc_pwtmpbuf
-			  && __libc_use_alloca (alloca_used
-						+ 2 * pwbuflen))
+		      v = size_add_wrapv (pwbuflen, pwbuflen, &newlen);
+		      if (!v && malloc_pwtmpbuf == NULL
+			  && glob_use_alloca (alloca_used, newlen))
 			pwtmpbuf = extend_alloca_account (pwtmpbuf, pwbuflen,
-							  2 * pwbuflen,
-							  alloca_used);
+							  newlen, alloca_used);
 		      else
 			{
-			  char *newp = realloc (malloc_pwtmpbuf
-						? pwtmpbuf : NULL,
-						2 * pwbuflen);
+			  char *newp = (v ? NULL
+					: realloc (malloc_pwtmpbuf, newlen));
 			  if (newp == NULL)
 			    {
-			      if (__glibc_unlikely (malloc_pwtmpbuf))
-				free (pwtmpbuf);
+			      free (malloc_pwtmpbuf);
+			      if (__glibc_unlikely (malloc_name))
+				free (name);
 			      retval = GLOB_NOSPACE;
 			      goto out;
 			    }
-			  pwtmpbuf = newp;
-			  pwbuflen = 2 * pwbuflen;
-			  malloc_pwtmpbuf = 1;
+			  malloc_pwtmpbuf = pwtmpbuf = newp;
 			}
+		      pwbuflen = newlen;
 		      __set_errno (save);
 		    }
-#   else
+# else
 		  p = getpwnam (name);
-#   endif
+# endif
+		  if (__glibc_unlikely (malloc_name))
+		    free (name);
 		  if (p != NULL)
 		    {
-		      if (!malloc_pwtmpbuf)
+		      if (malloc_pwtmpbuf == NULL)
 			home_dir = p->pw_dir;
 		      else
 			{
 			  size_t home_dir_len = strlen (p->pw_dir) + 1;
-			  if (__libc_use_alloca (alloca_used + home_dir_len))
+			  if (glob_use_alloca (alloca_used, home_dir_len))
 			    home_dir = alloca_account (home_dir_len,
 						       alloca_used);
 			  else
@@ -720,26 +740,32 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 			      malloc_home_dir = 1;
 			    }
 			  memcpy (home_dir, p->pw_dir, home_dir_len);
-
-			  free (pwtmpbuf);
 			}
 		    }
+		  free (malloc_pwtmpbuf);
+		}
+	      else
+		{
+		  if (__glibc_unlikely (malloc_name))
+		    free (name);
 		}
+#endif /* WINDOWS32 */
 	    }
 	  if (home_dir == NULL || home_dir[0] == '\0')
 	    {
+	      if (__glibc_unlikely (malloc_home_dir))
+		free (home_dir);
 	      if (flags & GLOB_TILDE_CHECK)
 		{
-		  if (__glibc_unlikely (malloc_home_dir))
-		    free (home_dir);
 		  retval = GLOB_NOMATCH;
 		  goto out;
 		}
 	      else
-		home_dir = (char *) "~"; /* No luck.  */
+		{
+		  home_dir = (char *) "~"; /* No luck.  */
+		  malloc_home_dir = 0;
+		}
 	    }
-#  endif /* WINDOWS32 */
-# endif
 	  /* Now construct the full directory.  */
 	  if (dirname[1] == '\0')
 	    {
@@ -754,8 +780,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	    {
 	      char *newp;
 	      size_t home_len = strlen (home_dir);
-	      int use_alloca = __libc_use_alloca (alloca_used
-						  + home_len + dirlen);
+	      int use_alloca = glob_use_alloca (alloca_used, home_len + dirlen);
 	      if (use_alloca)
 		newp = alloca_account (home_len + dirlen, alloca_used);
 	      else
@@ -779,12 +804,15 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      dirname = newp;
 	      dirlen += home_len - 1;
 	      malloc_dirname = !use_alloca;
+
+	      if (__glibc_unlikely (malloc_home_dir))
+		free (home_dir);
 	    }
 	  dirname_modified = 1;
 	}
-# if !defined _AMIGA && !defined WINDOWS32
       else
 	{
+#ifndef WINDOWS32
 	  char *end_name = strchr (dirname, '/');
 	  char *user_name;
 	  int malloc_user_name = 0;
@@ -806,7 +834,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  else
 	    {
 	      char *newp;
-	      if (__libc_use_alloca (alloca_used + (end_name - dirname)))
+	      if (glob_use_alloca (alloca_used, end_name - dirname))
 		newp = alloca_account (end_name - dirname, alloca_used);
 	      else
 		{
@@ -851,20 +879,21 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  /* Look up specific user's home directory.  */
 	  {
 	    struct passwd *p;
+	    char *malloc_pwtmpbuf = NULL;
 #  if defined HAVE_GETPWNAM_R || defined _LIBC
-	    long int buflen = GETPW_R_SIZE_MAX ();
+	    long int buflenmax = GETPW_R_SIZE_MAX ();
+	    size_t buflen = buflenmax;
 	    char *pwtmpbuf;
-	    int malloc_pwtmpbuf = 0;
 	    struct passwd pwbuf;
 	    int save = errno;
 
 #   ifndef _LIBC
-	    if (buflen == -1)
-	      /* `sysconf' does not support _SC_GETPW_R_SIZE_MAX.  Try a
+	    if (! (0 <= buflenmax && buflenmax <= SIZE_MAX))
+	      /* Perhaps 'sysconf' does not support _SC_GETPW_R_SIZE_MAX.  Try a
 		 moderate value.  */
 	      buflen = 1024;
 #   endif
-	    if (__libc_use_alloca (alloca_used + buflen))
+	    if (glob_use_alloca (alloca_used, buflen))
 	      pwtmpbuf = alloca_account (buflen, alloca_used);
 	    else
 	      {
@@ -877,32 +906,32 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		    retval = GLOB_NOSPACE;
 		    goto out;
 		  }
-		malloc_pwtmpbuf = 1;
+		malloc_pwtmpbuf = pwtmpbuf;
 	      }
 
 	    while (getpwnam_r (user_name, &pwbuf, pwtmpbuf, buflen, &p) != 0)
 	      {
+		size_t newlen;
+		bool v;
 		if (errno != ERANGE)
 		  {
 		    p = NULL;
 		    break;
 		  }
-		if (!malloc_pwtmpbuf
-		    && __libc_use_alloca (alloca_used + 2 * buflen))
+		v = size_add_wrapv (buflen, buflen, &newlen);
+		if (!v && malloc_pwtmpbuf == NULL
+		    && glob_use_alloca (alloca_used, newlen))
 		  pwtmpbuf = extend_alloca_account (pwtmpbuf, buflen,
-						    2 * buflen, alloca_used);
+						    newlen, alloca_used);
 		else
 		  {
-		    char *newp = realloc (malloc_pwtmpbuf ? pwtmpbuf : NULL,
-					  2 * buflen);
+		    char *newp = v ? NULL : realloc (malloc_pwtmpbuf, newlen);
 		    if (newp == NULL)
 		      {
-			if (__glibc_unlikely (malloc_pwtmpbuf))
-			  free (pwtmpbuf);
+			free (malloc_pwtmpbuf);
 			goto nomem_getpw;
 		      }
-		    pwtmpbuf = newp;
-		    malloc_pwtmpbuf = 1;
+		    malloc_pwtmpbuf = pwtmpbuf = newp;
 		  }
 		__set_errno (save);
 	      }
@@ -923,7 +952,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		  free (dirname);
 		malloc_dirname = 0;
 
-		if (__libc_use_alloca (alloca_used + home_len + rest_len + 1))
+		if (glob_use_alloca (alloca_used, home_len + rest_len + 1))
 		  dirname = alloca_account (home_len + rest_len + 1,
 					    alloca_used);
 		else
@@ -931,8 +960,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		    dirname = malloc (home_len + rest_len + 1);
 		    if (dirname == NULL)
 		      {
-			if (__glibc_unlikely (malloc_pwtmpbuf))
-			  free (pwtmpbuf);
+			free (malloc_pwtmpbuf);
 			retval = GLOB_NOSPACE;
 			goto out;
 		      }
@@ -944,24 +972,24 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		dirlen = home_len + rest_len;
 		dirname_modified = 1;
 
-		if (__glibc_unlikely (malloc_pwtmpbuf))
-		  free (pwtmpbuf);
+		free (malloc_pwtmpbuf);
 	      }
 	    else
 	      {
-		if (__glibc_unlikely (malloc_pwtmpbuf))
-		  free (pwtmpbuf);
+		free (malloc_pwtmpbuf);
 
 		if (flags & GLOB_TILDE_CHECK)
-		  /* We have to regard it as an error if we cannot find the
-		     home directory.  */
-		  return GLOB_NOMATCH;
+		  {
+		    /* We have to regard it as an error if we cannot find the
+		       home directory.  */
+		    retval = GLOB_NOMATCH;
+		    goto out;
+		  }
 	      }
 	  }
+#endif /* !WINDOWS32 */
 	}
-# endif	/* Not Amiga && not WINDOWS32.  */
     }
-#endif	/* Not VMS.  */
 
   /* Now test whether we looked for "~" or "~NAME".  In this case we
      can give the answer now.  */
@@ -980,19 +1008,18 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  size_t newcount = pglob->gl_pathc + pglob->gl_offs;
 	  char **new_gl_pathv;
 
-	  if (newcount > UINTPTR_MAX - (1 + 1)
-	      || newcount + 1 + 1 > ~((size_t) 0) / sizeof (char *))
+	  if (newcount > SIZE_MAX / sizeof (char *) - 2)
 	    {
 	    nospace:
 	      free (pglob->gl_pathv);
 	      pglob->gl_pathv = NULL;
 	      pglob->gl_pathc = 0;
-	      return GLOB_NOSPACE;
+	      retval = GLOB_NOSPACE;
+	      goto out;
 	    }
 
-	  new_gl_pathv
-	    = (char **) realloc (pglob->gl_pathv,
-				 (newcount + 1 + 1) * sizeof (char *));
+	  new_gl_pathv = realloc (pglob->gl_pathv,
+				  (newcount + 2) * sizeof (char *));
 	  if (new_gl_pathv == NULL)
 	    goto nospace;
 	  pglob->gl_pathv = new_gl_pathv;
@@ -1006,12 +1033,19 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      p = mempcpy (pglob->gl_pathv[newcount], dirname, dirlen);
 	      p[0] = '/';
 	      p[1] = '\0';
+	      if (__glibc_unlikely (malloc_dirname))
+		free (dirname);
 	    }
 	  else
 	    {
-	      pglob->gl_pathv[newcount] = strdup (dirname);
-	      if (pglob->gl_pathv[newcount] == NULL)
-		goto nospace;
+	      if (__glibc_unlikely (malloc_dirname))
+		pglob->gl_pathv[newcount] = dirname;
+	      else
+		{
+		  pglob->gl_pathv[newcount] = strdup (dirname);
+		  if (pglob->gl_pathv[newcount] == NULL)
+		    goto nospace;
+		}
 	    }
 	  pglob->gl_pathv[++newcount] = NULL;
 	  ++pglob->gl_pathc;
@@ -1021,7 +1055,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	}
 
       /* Not found.  */
-      return GLOB_NOMATCH;
+      retval = GLOB_NOMATCH;
+      goto out;
     }
 
   meta = __glob_pattern_type (dirname, !(flags & GLOB_NOESCAPE));
@@ -1067,7 +1102,10 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
       if (status != 0)
 	{
 	  if ((flags & GLOB_NOCHECK) == 0 || status != GLOB_NOMATCH)
-	    return status;
+	    {
+	      retval = status;
+	      goto out;
+	    }
 	  goto no_matches;
 	}
 
@@ -1078,19 +1116,6 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	{
 	  size_t old_pathc;
 
-#ifdef	SHELL
-	  {
-	    /* Make globbing interruptible in the bash shell. */
-	    extern int interrupt_state;
-
-	    if (interrupt_state)
-	      {
-		globfree (&dirs);
-		return GLOB_ABORTED;
-	      }
-	  }
-#endif /* SHELL.  */
-
 	  old_pathc = pglob->gl_pathc;
 	  status = glob_in_dir (filename, dirs.gl_pathv[i],
 				((flags | GLOB_APPEND)
@@ -1105,7 +1130,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      globfree (&dirs);
 	      globfree (pglob);
 	      pglob->gl_pathc = 0;
-	      return status;
+	      retval = status;
+	      goto out;
 	    }
 
 	  /* Stick the directory on the front of each name.  */
@@ -1116,13 +1142,14 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      globfree (&dirs);
 	      globfree (pglob);
 	      pglob->gl_pathc = 0;
-	      return GLOB_NOSPACE;
+	      retval = GLOB_NOSPACE;
+	      goto out;
 	    }
 	}
 
       flags |= GLOB_MAGCHAR;
 
-      /* We have ignored the GLOB_NOCHECK flag in the `glob_in_dir' calls.
+      /* We have ignored the GLOB_NOCHECK flag in the 'glob_in_dir' calls.
 	 But if we have not found any matching entry and the GLOB_NOCHECK
 	 flag was set we must return the input pattern itself.  */
       if (pglob->gl_pathc + pglob->gl_offs == oldcount)
@@ -1134,28 +1161,28 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      size_t newcount = pglob->gl_pathc + pglob->gl_offs;
 	      char **new_gl_pathv;
 
-	      if (newcount > UINTPTR_MAX - 2
-		  || newcount + 2 > ~((size_t) 0) / sizeof (char *))
+	      if (newcount > SIZE_MAX / sizeof (char *) - 2)
 		{
 		nospace2:
 		  globfree (&dirs);
-		  return GLOB_NOSPACE;
+		  retval = GLOB_NOSPACE;
+		  goto out;
 		}
 
-	      new_gl_pathv = (char **) realloc (pglob->gl_pathv,
-						(newcount + 2)
-						* sizeof (char *));
+	      new_gl_pathv = realloc (pglob->gl_pathv,
+				      (newcount + 2) * sizeof (char *));
 	      if (new_gl_pathv == NULL)
 		goto nospace2;
 	      pglob->gl_pathv = new_gl_pathv;
 
-	      pglob->gl_pathv[newcount] = __strdup (pattern);
+	      pglob->gl_pathv[newcount] = strdup (pattern);
 	      if (pglob->gl_pathv[newcount] == NULL)
 		{
 		  globfree (&dirs);
 		  globfree (pglob);
 		  pglob->gl_pathc = 0;
-		  return GLOB_NOSPACE;
+		  retval = GLOB_NOSPACE;
+		  goto out;
 		}
 
 	      ++pglob->gl_pathc;
@@ -1167,7 +1194,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  else
 	    {
 	      globfree (&dirs);
-	      return GLOB_NOMATCH;
+	      retval = GLOB_NOMATCH;
+	      goto out;
 	    }
 	}
 
@@ -1213,7 +1241,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      flags = orig_flags;
 	      goto no_matches;
 	    }
-	  return status;
+	  retval = status;
+	  goto out;
 	}
 
       if (dirlen > 0)
@@ -1225,7 +1254,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	    {
 	      globfree (pglob);
 	      pglob->gl_pathc = 0;
-	      return GLOB_NOSPACE;
+	      retval = GLOB_NOSPACE;
+	      goto out;
 	    }
 	}
     }
@@ -1250,7 +1280,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      {
 		globfree (pglob);
 		pglob->gl_pathc = 0;
-		return GLOB_NOSPACE;
+		retval = GLOB_NOSPACE;
+		goto out;
 	      }
 	    strcpy (&new[len - 2], "/");
 	    pglob->gl_pathv[i] = new;
@@ -1276,32 +1307,12 @@ libc_hidden_def (glob)
 #endif
 
 
-#if !defined _LIBC || !defined GLOB_ONLY_P
-
-/* Free storage allocated in PGLOB by a previous `glob' call.  */
-void
-globfree (glob_t *pglob)
-{
-  if (pglob->gl_pathv != NULL)
-    {
-      size_t i;
-      for (i = 0; i < pglob->gl_pathc; ++i)
-	free (pglob->gl_pathv[pglob->gl_offs + i]);
-      free (pglob->gl_pathv);
-      pglob->gl_pathv = NULL;
-    }
-}
-#if defined _LIBC && !defined globfree
-libc_hidden_def (globfree)
-#endif
-
-
 /* Do a collated comparison of A and B.  */
 static int
 collated_compare (const void *a, const void *b)
 {
-  const char *const s1 = *(const char *const * const) a;
-  const char *const s2 = *(const char *const * const) b;
+  char *const *ps1 = a; char *s1 = *ps1;
+  char *const *ps2 = b; char *s2 = *ps2;
 
   if (s1 == s2)
     return 0;
@@ -1322,28 +1333,24 @@ prefix_array (const char *dirname, char **array, size_t n)
 {
   size_t i;
   size_t dirlen = strlen (dirname);
-#if defined __MSDOS__ || defined WINDOWS32
-  int sep_char = '/';
-# define DIRSEP_CHAR sep_char
-#else
-# define DIRSEP_CHAR '/'
-#endif
+  char dirsep_char = '/';
 
   if (dirlen == 1 && dirname[0] == '/')
     /* DIRNAME is just "/", so normal prepending would get us "//foo".
        We want "/foo" instead, so don't prepend any chars from DIRNAME.  */
     dirlen = 0;
+
 #if defined __MSDOS__ || defined WINDOWS32
-  else if (dirlen > 1)
+  if (dirlen > 1)
     {
       if (dirname[dirlen - 1] == '/' && dirname[dirlen - 2] == ':')
 	/* DIRNAME is "d:/".  Don't prepend the slash from DIRNAME.  */
 	--dirlen;
       else if (dirname[dirlen - 1] == ':')
 	{
-	  /* DIRNAME is "d:".  Use `:' instead of `/'.  */
+	  /* DIRNAME is "d:".  Use ':' instead of '/'.  */
 	  --dirlen;
-	  sep_char = ':';
+	  dirsep_char = ':';
 	}
     }
 #endif
@@ -1351,7 +1358,7 @@ prefix_array (const char *dirname, char **array, size_t n)
   for (i = 0; i < n; ++i)
     {
       size_t eltlen = strlen (array[i]) + 1;
-      char *new = (char *) malloc (dirlen + 1 + eltlen);
+      char *new = malloc (dirlen + 1 + eltlen);
       if (new == NULL)
 	{
 	  while (i > 0)
@@ -1361,7 +1368,7 @@ prefix_array (const char *dirname, char **array, size_t n)
 
       {
 	char *endp = mempcpy (new, dirname, dirlen);
-	*endp++ = DIRSEP_CHAR;
+	*endp++ = dirsep_char;
 	mempcpy (endp, array[i], eltlen);
       }
       free (array[i]);
@@ -1371,103 +1378,57 @@ prefix_array (const char *dirname, char **array, size_t n)
   return 0;
 }
 
-
-/* We must not compile this function twice.  */
-#if !defined _LIBC || !defined NO_GLOB_PATTERN_P
-int
-__glob_pattern_type (const char *pattern, int quote)
-{
-  const char *p;
-  int ret = 0;
-
-  for (p = pattern; *p != '\0'; ++p)
-    switch (*p)
-      {
-      case '?':
-      case '*':
-	return 1;
-
-      case '\\':
-	if (quote)
-	  {
-	    if (p[1] != '\0')
-	      ++p;
-	    ret |= 2;
-	  }
-	break;
-
-      case '[':
-	ret |= 4;
-	break;
-
-      case ']':
-	if (ret & 4)
-	  return 1;
-	break;
-      }
-
-  return ret;
-}
-
-/* Return nonzero if PATTERN contains any metacharacters.
-   Metacharacters can be quoted with backslashes if QUOTE is nonzero.  */
-int
-__glob_pattern_p (const char *pattern, int quote)
-{
-  return __glob_pattern_type (pattern, quote) == 1;
-}
-# ifdef _LIBC
-weak_alias (__glob_pattern_p, glob_pattern_p)
-# endif
-#endif
-
-#endif /* !GLOB_ONLY_P */
-
-
 /* We put this in a separate function mainly to allow the memory
    allocated with alloca to be recycled.  */
-#if !defined _LIBC || !defined GLOB_ONLY_P
 static int
 __attribute_noinline__
-link_exists2_p (const char *dir, size_t dirlen, const char *fname,
-	       glob_t *pglob
-# ifndef _LIBC
-		, int flags
+link_stat (const char *dir, size_t dirlen, const char *fname,
+	   glob_t *pglob
+# if !defined _LIBC && !HAVE_FSTATAT
+	   , int flags
 # endif
-		)
+	   )
 {
   size_t fnamelen = strlen (fname);
-  char *fullname = (char *) __alloca (dirlen + 1 + fnamelen + 1);
+  char *fullname = __alloca (dirlen + 1 + fnamelen + 1);
   struct stat st;
-# ifndef _LIBC
-  struct_stat64 st64;
-# endif
 
   mempcpy (mempcpy (mempcpy (fullname, dir, dirlen), "/", 1),
 	   fname, fnamelen + 1);
 
-# ifdef _LIBC
-  return (*pglob->gl_stat) (fullname, &st) == 0;
-# else
-  return ((__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
-	   ? (*pglob->gl_stat) (fullname, &st)
-	   : __stat64 (fullname, &st64)) == 0);
+# if !defined _LIBC && !HAVE_FSTATAT
+  if (__builtin_expect ((flags & GLOB_ALTDIRFUNC) == 0, 1))
+    {
+      struct_stat64 st64;
+      return __stat64 (fullname, &st64);
+    }
 # endif
+  return (*pglob->gl_stat) (fullname, &st);
 }
-# ifdef _LIBC
-#  define link_exists_p(dfd, dirname, dirnamelen, fname, pglob, flags) \
-  (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)			      \
-   ? link_exists2_p (dirname, dirnamelen, fname, pglob)			      \
-   : ({ struct stat64 st64;						      \
-       __fxstatat64 (_STAT_VER, dfd, fname, &st64, 0) == 0; }))
+
+/* Return true if DIR/FNAME exists.  */
+static int
+link_exists_p (int dfd, const char *dir, size_t dirlen, const char *fname,
+	       glob_t *pglob, int flags)
+{
+  int status;
+# if defined _LIBC || HAVE_FSTATAT
+  if (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0))
+    status = link_stat (dir, dirlen, fname, pglob);
+  else
+    {
+      /* dfd cannot be -1 here, because dirfd never returns -1 on
+	 glibc, or on hosts that have fstatat.  */
+      struct_stat64 st64;
+      status = __fxstatat64 (_STAT_VER, dfd, fname, &st64, 0);
+    }
 # else
-#  define link_exists_p(dfd, dirname, dirnamelen, fname, pglob, flags) \
-  link_exists2_p (dirname, dirnamelen, fname, pglob, flags)
+  status = link_stat (dir, dirlen, fname, pglob, flags);
 # endif
-#endif
-
+  return status == 0 || errno == EOVERFLOW;
+}
 
-/* Like `glob', but PATTERN is a final pathname component,
+/* Like 'glob', but PATTERN is a final pathname component,
    and matches are searched for in DIRECTORY.
    The GLOB_NOSORT bit in FLAGS is ignored.  No sorting is ever done.
    The GLOB_APPEND flag is assumed to be set (always appends).  */
@@ -1478,25 +1439,25 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 {
   size_t dirlen = strlen (directory);
   void *stream = NULL;
-  struct globnames
-    {
-      struct globnames *next;
-      size_t count;
-      char *name[64];
-    };
-#define INITIAL_COUNT sizeof (init_names.name) / sizeof (init_names.name[0])
-  struct globnames init_names;
-  struct globnames *names = &init_names;
-  struct globnames *names_alloca = &init_names;
+# define GLOBNAMES_MEMBERS(nnames) \
+    struct globnames *next; size_t count; char *name[nnames];
+  struct globnames { GLOBNAMES_MEMBERS (FLEXIBLE_ARRAY_MEMBER) };
+  struct { GLOBNAMES_MEMBERS (64) } init_names_buf;
+  struct globnames *init_names = (struct globnames *) &init_names_buf;
+  struct globnames *names = init_names;
+  struct globnames *names_alloca = init_names;
   size_t nfound = 0;
   size_t cur = 0;
   int meta;
   int save;
+  int result;
 
-  alloca_used += sizeof (init_names);
+  alloca_used += sizeof init_names_buf;
 
-  init_names.next = NULL;
-  init_names.count = INITIAL_COUNT;
+  init_names->next = NULL;
+  init_names->count = ((sizeof init_names_buf
+                        - offsetof (struct globnames, name))
+                       / sizeof init_names->name[0]);
 
   meta = __glob_pattern_type (pattern, !(flags & GLOB_NOESCAPE));
   if (meta == 0 && (flags & (GLOB_NOCHECK|GLOB_NOMAGIC)))
@@ -1516,14 +1477,16 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 	struct_stat64 st64;
       } ust;
       size_t patlen = strlen (pattern);
-      int alloca_fullname = __libc_use_alloca (alloca_used
-					       + dirlen + 1 + patlen + 1);
+      size_t fullsize;
+      bool alloca_fullname
+        = (! size_add_wrapv (dirlen + 1, patlen + 1, &fullsize)
+           && glob_use_alloca (alloca_used, fullsize));
       char *fullname;
       if (alloca_fullname)
-	fullname = alloca_account (dirlen + 1 + patlen + 1, alloca_used);
+        fullname = alloca_account (fullsize, alloca_used);
       else
 	{
-	  fullname = malloc (dirlen + 1 + patlen + 1);
+	  fullname = malloc (fullsize);
 	  if (fullname == NULL)
 	    return GLOB_NOSPACE;
 	}
@@ -1531,9 +1494,11 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
       mempcpy (mempcpy (mempcpy (fullname, directory, dirlen),
 			"/", 1),
 	       pattern, patlen + 1);
-      if ((__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
+      if (((__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
 	   ? (*pglob->gl_stat) (fullname, &ust.st)
-	   : __stat64 (fullname, &ust.st64)) == 0)
+	    : __stat64 (fullname, &ust.st64))
+	   == 0)
+	  || errno == EOVERFLOW)
 	/* We found this file to be existing.  Now tell the rest
 	   of the function to copy this name into the result.  */
 	flags |= GLOB_NOCHECK;
@@ -1555,16 +1520,10 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 	}
       else
 	{
-#ifdef _LIBC
 	  int dfd = (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
 		     ? -1 : dirfd ((DIR *) stream));
-#endif
 	  int fnm_flags = ((!(flags & GLOB_PERIOD) ? FNM_PERIOD : 0)
-			   | ((flags & GLOB_NOESCAPE) ? FNM_NOESCAPE : 0)
-#if defined _AMIGA || defined VMS
-			   | FNM_CASEFOLD
-#endif
-			   );
+			   | ((flags & GLOB_NOESCAPE) ? FNM_NOESCAPE : 0));
 	  flags |= GLOB_MAGCHAR;
 
 	  while (1)
@@ -1584,19 +1543,24 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 	      }
 	      if (d.name == NULL)
 		break;
-	      if (d.skip_entry)
+	      if (readdir_result_skip_entry (d))
 		continue;
 
 	      /* If we shall match only directories use the information
 		 provided by the dirent call if possible.  */
-	      if ((flags & GLOB_ONLYDIR) && !readdir_result_might_be_dir (d))
-		continue;
+	      if (flags & GLOB_ONLYDIR)
+		switch (readdir_result_type (d))
+		  {
+		  case DT_DIR: case DT_LNK: case DT_UNKNOWN: break;
+		  default: continue;
+		  }
 
 	      if (fnmatch (pattern, d.name, fnm_flags) == 0)
 		{
 		  /* If the file we found is a symlink we have to
 		     make sure the target file exists.  */
-		  if (!readdir_result_might_be_symlink (d)
+		  dirent_type type = readdir_result_type (d);
+		  if (! (type == DT_LNK || type == DT_UNKNOWN)
 		      || link_exists_p (dfd, directory, dirlen, d.name,
 					pglob, flags))
 		    {
@@ -1604,10 +1568,13 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 			{
 			  struct globnames *newnames;
 			  size_t count = names->count * 2;
-			  size_t size = (sizeof (struct globnames)
-					 + ((count - INITIAL_COUNT)
-					    * sizeof (char *)));
-			  if (__libc_use_alloca (alloca_used + size))
+			  size_t nameoff = offsetof (struct globnames, name);
+			  size_t size = FLEXSIZEOF (struct globnames, name,
+						    count * sizeof (char *));
+			  if ((SIZE_MAX - nameoff) / 2 / sizeof (char *)
+			      < names->count)
+			    goto memory_error;
+			  if (glob_use_alloca (alloca_used, size))
 			    newnames = names_alloca
 			      = alloca_account (size, alloca_used);
 			  else if ((newnames = malloc (size))
@@ -1623,6 +1590,8 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 			goto memory_error;
 		      ++cur;
 		      ++nfound;
+		      if (SIZE_MAX - pglob->gl_offs <= nfound)
+			goto memory_error;
 		    }
 		}
 	    }
@@ -1633,29 +1602,27 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
     {
       size_t len = strlen (pattern);
       nfound = 1;
-      names->name[cur] = (char *) malloc (len + 1);
+      names->name[cur] = malloc (len + 1);
       if (names->name[cur] == NULL)
 	goto memory_error;
       *((char *) mempcpy (names->name[cur++], pattern, len)) = '\0';
     }
 
-  int result = GLOB_NOMATCH;
+  result = GLOB_NOMATCH;
   if (nfound != 0)
     {
+      char **new_gl_pathv;
       result = 0;
 
-      if (pglob->gl_pathc > UINTPTR_MAX - pglob->gl_offs
-	  || pglob->gl_pathc + pglob->gl_offs > UINTPTR_MAX - nfound
-	  || pglob->gl_pathc + pglob->gl_offs + nfound > UINTPTR_MAX - 1
-	  || (pglob->gl_pathc + pglob->gl_offs + nfound + 1
-	      > UINTPTR_MAX / sizeof (char *)))
+      if (SIZE_MAX / sizeof (char *) - pglob->gl_pathc
+	  < pglob->gl_offs + nfound + 1)
 	goto memory_error;
 
-      char **new_gl_pathv;
       new_gl_pathv
-	= (char **) realloc (pglob->gl_pathv,
-			     (pglob->gl_pathc + pglob->gl_offs + nfound + 1)
-			     * sizeof (char *));
+	= realloc (pglob->gl_pathv,
+		   (pglob->gl_pathc + pglob->gl_offs + nfound + 1)
+		    * sizeof (char *));
+
       if (new_gl_pathv == NULL)
 	{
 	memory_error:
@@ -1671,7 +1638,7 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 		 and this is the block assigned to OLD here.  */
 	      if (names == NULL)
 		{
-		  assert (old == &init_names);
+		  assert (old == init_names);
 		  break;
 		}
 	      cur = names->count;
@@ -1697,7 +1664,7 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 		 and this is the block assigned to OLD here.  */
 	      if (names == NULL)
 		{
-		  assert (old == &init_names);
+		  assert (old == init_names);
 		  break;
 		}
 	      cur = names->count;
diff --git a/posix/glob64.c b/posix/glob64.c
index 6cb3d65..a515a1c 100644
--- a/posix/glob64.c
+++ b/posix/glob64.c
@@ -43,10 +43,4 @@ glob64 (const char *pattern, int flags,
 }
 libc_hidden_def (glob64)
 
-void
-globfree64 (glob64_t *pglob)
-{
-}
-libc_hidden_def (globfree64)
-
 stub_warning (glob64)
diff --git a/posix/glob_internal.h b/posix/glob_internal.h
new file mode 100644
index 0000000..12c9366
--- /dev/null
+++ b/posix/glob_internal.h
@@ -0,0 +1,57 @@
+/* Shared definition for glob and glob_pattern_p.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef GLOB_INTERNAL_H
+# define GLOB_INTERNAL_H
+
+static inline int
+__glob_pattern_type (const char *pattern, int quote)
+{
+  const char *p;
+  int ret = 0;
+
+  for (p = pattern; *p != '\0'; ++p)
+    switch (*p)
+      {
+      case '?':
+      case '*':
+        return 1;
+
+      case '\\':
+        if (quote)
+          {
+            if (p[1] != '\0')
+              ++p;
+            ret |= 2;
+          }
+        break;
+
+      case '[':
+        ret |= 4;
+        break;
+
+      case ']':
+        if (ret & 4)
+          return 1;
+        break;
+      }
+
+  return ret;
+}
+
+#endif /* GLOB_INTERNAL_H  */
diff --git a/posix/glob_pattern_p.c b/posix/glob_pattern_p.c
new file mode 100644
index 0000000..a17d337
--- /dev/null
+++ b/posix/glob_pattern_p.c
@@ -0,0 +1,33 @@
+/* Return nonzero if PATTERN contains any metacharacters.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _LIBC
+# include <config.h>
+#endif
+
+#include <glob.h>
+#include "glob_internal.h"
+
+/* Return nonzero if PATTERN contains any metacharacters.
+   Metacharacters can be quoted with backslashes if QUOTE is nonzero.  */
+int
+__glob_pattern_p (const char *pattern, int quote)
+{
+  return __glob_pattern_type (pattern, quote) == 1;
+}
+weak_alias (__glob_pattern_p, glob_pattern_p)
diff --git a/posix/globfree.c b/posix/globfree.c
new file mode 100644
index 0000000..042e29d
--- /dev/null
+++ b/posix/globfree.c
@@ -0,0 +1,41 @@
+/* Frees the dynamically allocated storage from an earlier call to glob.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _LIBC
+# include <config.h>
+#endif
+
+#include <glob.h>
+#include <stdlib.h>
+
+/* Free storage allocated in PGLOB by a previous `glob' call.  */
+void
+globfree (glob_t *pglob)
+{
+  if (pglob->gl_pathv != NULL)
+    {
+      size_t i;
+      for (i = 0; i < pglob->gl_pathc; ++i)
+        free (pglob->gl_pathv[pglob->gl_offs + i]);
+      free (pglob->gl_pathv);
+      pglob->gl_pathv = NULL;
+    }
+}
+#ifndef globfree
+libc_hidden_def (globfree)
+#endif
diff --git a/posix/globfree64.c b/posix/globfree64.c
new file mode 100644
index 0000000..c9f8908
--- /dev/null
+++ b/posix/globfree64.c
@@ -0,0 +1,31 @@
+/* Frees the dynamically allocated storage from an earlier call to glob.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _LIBC
+# include <config.h>
+#endif
+
+#include <glob.h>
+#include <stdlib.h>
+
+/* Free storage allocated in PGLOB by a previous `glob' call.  */
+void
+globfree64 (glob64_t *pglob)
+{
+}
+libc_hidden_def (globfree64)
diff --git a/sysdeps/gnu/glob64.c b/sysdeps/gnu/glob64.c
index d1e4e6f..52e97e2 100644
--- a/sysdeps/gnu/glob64.c
+++ b/sysdeps/gnu/glob64.c
@@ -15,11 +15,8 @@
 #undef __stat
 #define __stat(file, buf) __xstat64 (_STAT_VER, file, buf)
 
-#define NO_GLOB_PATTERN_P 1
-
 #define COMPILE_GLOB64	1
 
 #include <posix/glob.c>
 
 libc_hidden_def (glob64)
-libc_hidden_def (globfree64)
diff --git a/sysdeps/gnu/globfree64.c b/sysdeps/gnu/globfree64.c
new file mode 100644
index 0000000..f092d0b
--- /dev/null
+++ b/sysdeps/gnu/globfree64.c
@@ -0,0 +1,10 @@
+#include <dirent.h>
+#include <glob.h>
+#include <sys/stat.h>
+
+#define glob_t glob64_t
+#define globfree(pglob) globfree64 (pglob)
+
+#include <posix/globfree.c>
+
+libc_hidden_def (globfree64)
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index e571fe2..30bd167 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -137,7 +137,7 @@ endif
 ifeq ($(subdir),posix)
 sysdep_headers += bits/initspin.h
 
-sysdep_routines += sched_getcpu
+sysdep_routines += sched_getcpu oldglob
 
 tests += tst-affinity tst-affinity-pid
 
diff --git a/sysdeps/unix/sysv/linux/alpha/glob.c b/sysdeps/unix/sysv/linux/alpha/glob.c
index 2d7d287..1b813c1 100644
--- a/sysdeps/unix/sysv/linux/alpha/glob.c
+++ b/sysdeps/unix/sysv/linux/alpha/glob.c
@@ -42,10 +42,6 @@ extern void __new_globfree (glob_t *__pglob);
 #undef globfree64
 
 versioned_symbol (libc, __new_glob, glob, GLIBC_2_1);
-versioned_symbol (libc, __new_globfree, globfree, GLIBC_2_1);
 libc_hidden_ver (__new_glob, glob)
-libc_hidden_ver (__new_globfree, globfree)
 
 weak_alias (__new_glob, glob64)
-weak_alias (__new_globfree, globfree64)
-libc_hidden_ver (__new_globfree, globfree64)
diff --git a/sysdeps/unix/sysv/linux/alpha/globfree.c b/sysdeps/unix/sysv/linux/alpha/globfree.c
new file mode 100644
index 0000000..98cf1c2
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/alpha/globfree.c
@@ -0,0 +1,37 @@
+/* Compat globfree.  Linux/alpha version.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library.  If not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#define globfree64 __no_globfree64_decl
+#include <sys/types.h>
+#include <glob.h>
+#include <shlib-compat.h>
+
+#define globfree(pglob) \
+  __new_globfree (pglob)
+
+extern void __new_globfree (glob_t *__pglob);
+
+#include <posix/globfree.c>
+
+#undef globfree64
+
+versioned_symbol (libc, __new_globfree, globfree, GLIBC_2_1);
+libc_hidden_ver (__new_globfree, globfree)
+
+weak_alias (__new_globfree, globfree64)
+libc_hidden_ver (__new_globfree, globfree64)
diff --git a/sysdeps/unix/sysv/linux/i386/glob64.c b/sysdeps/unix/sysv/linux/i386/glob64.c
index 956cb04..230f9fc 100644
--- a/sysdeps/unix/sysv/linux/i386/glob64.c
+++ b/sysdeps/unix/sysv/linux/i386/glob64.c
@@ -19,6 +19,7 @@
 #include <dirent.h>
 #include <glob.h>
 #include <sys/stat.h>
+#include <shlib-compat.h>
 
 #define dirent dirent64
 #define __readdir(dirp) __readdir64 (dirp)
@@ -33,47 +34,9 @@
 #undef __stat
 #define __stat(file, buf) __xstat64 (_STAT_VER, file, buf)
 
-#define NO_GLOB_PATTERN_P 1
-
 #define COMPILE_GLOB64	1
 
 #include <posix/glob.c>
 
-#include "shlib-compat.h"
-
-libc_hidden_def (globfree64)
-
 versioned_symbol (libc, __glob64, glob64, GLIBC_2_2);
 libc_hidden_ver (__glob64, glob64)
-
-#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
-
-#include <sysdeps/unix/sysv/linux/i386/olddirent.h>
-
-int __old_glob64 (const char *__pattern, int __flags,
-		  int (*__errfunc) (const char *, int),
-		  glob64_t *__pglob);
-libc_hidden_proto (__old_glob64);
-
-#undef dirent
-#define dirent __old_dirent64
-#undef GL_READDIR
-# define GL_READDIR(pglob, stream) \
-  ((struct __old_dirent64 *) (pglob)->gl_readdir (stream))
-#undef __readdir
-#define __readdir(dirp) __old_readdir64 (dirp)
-#undef glob
-#define glob(pattern, flags, errfunc, pglob) \
-  __old_glob64 (pattern, flags, errfunc, pglob)
-#define convert_dirent __old_convert_dirent
-#define glob_in_dir __old_glob_in_dir
-#define GLOB_ATTRIBUTE attribute_compat_text_section
-
-#define GLOB_ONLY_P 1
-
-#include <posix/glob.c>
-
-libc_hidden_def (__old_glob64);
-
-compat_symbol (libc, __old_glob64, glob64, GLIBC_2_1);
-#endif
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/globfree64.c b/sysdeps/unix/sysv/linux/mips/mips64/n64/globfree64.c
new file mode 100644
index 0000000..abc35fd
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/mips/mips64/n64/globfree64.c
@@ -0,0 +1 @@
+/* glob64 is in globfree64.c */
diff --git a/sysdeps/unix/sysv/linux/oldglob.c b/sysdeps/unix/sysv/linux/oldglob.c
new file mode 100644
index 0000000..8233e57
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/oldglob.c
@@ -0,0 +1,42 @@
+#include <shlib-compat.h>
+
+#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
+
+#include <dirent.h>
+#include <glob.h>
+#include <sys/stat.h>
+
+#include <sysdeps/unix/sysv/linux/i386/olddirent.h>
+
+int __old_glob64 (const char *__pattern, int __flags,
+		  int (*__errfunc) (const char *, int),
+		  glob64_t *__pglob);
+libc_hidden_proto (__old_glob64);
+
+#define dirent __old_dirent64
+#define GL_READDIR(pglob, stream) \
+  ((struct __old_dirent64 *) (pglob)->gl_readdir (stream))
+#undef __readdir
+#define __readdir(dirp) __old_readdir64 (dirp)
+
+#define glob_t glob64_t
+#define glob(pattern, flags, errfunc, pglob) \
+  __old_glob64 (pattern, flags, errfunc, pglob)
+#define globfree(pglob) globfree64(pglob)
+
+#define convert_dirent __old_convert_dirent
+#define glob_in_dir __old_glob_in_dir
+
+#undef stat
+#define stat stat64
+#undef __stat
+#define __stat(file, buf) __xstat64 (_STAT_VER, file, buf)
+
+#define GLOB_ATTRIBUTE attribute_compat_text_section
+
+#include <posix/glob.c>
+
+libc_hidden_def (__old_glob64);
+
+compat_symbol (libc, __old_glob64, glob64, GLIBC_2_1);
+#endif
diff --git a/sysdeps/unix/sysv/linux/wordsize-64/globfree64.c b/sysdeps/unix/sysv/linux/wordsize-64/globfree64.c
new file mode 100644
index 0000000..af035e1
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/wordsize-64/globfree64.c
@@ -0,0 +1,2 @@
+/* This file is here so sysdeps/gnu/glob64.c doesn't take precedence.  */
+#include <sysdeps/wordsize-64/globfree64.c>
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/globfree.c b/sysdeps/unix/sysv/linux/x86_64/x32/globfree.c
new file mode 100644
index 0000000..b76a761
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86_64/x32/globfree.c
@@ -0,0 +1 @@
+#include <sysdeps/wordsize-64/globfree.c>
diff --git a/sysdeps/wordsize-64/glob.c b/sysdeps/wordsize-64/glob.c
index 082faf1..954e8d3 100644
--- a/sysdeps/wordsize-64/glob.c
+++ b/sysdeps/wordsize-64/glob.c
@@ -4,5 +4,3 @@
 #undef glob64
 #undef globfree64
 weak_alias (glob, glob64)
-weak_alias (globfree, globfree64)
-libc_hidden_ver (globfree, globfree64)
diff --git a/sysdeps/wordsize-64/globfree.c b/sysdeps/wordsize-64/globfree.c
new file mode 100644
index 0000000..ec8c35b
--- /dev/null
+++ b/sysdeps/wordsize-64/globfree.c
@@ -0,0 +1,5 @@
+#define globfree64 __no_globfree64_decl
+#include <posix/globfree.c>
+#undef globfree64
+weak_alias (globfree, globfree64)
+libc_hidden_ver (globfree, globfree64)
diff --git a/sysdeps/wordsize-64/globfree64.c b/sysdeps/wordsize-64/globfree64.c
new file mode 100644
index 0000000..a0f57ff
--- /dev/null
+++ b/sysdeps/wordsize-64/globfree64.c
@@ -0,0 +1 @@
+/* globfree64 is in globfree.c */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 0/9] posix: glob fixes and refactor
@ 2017-09-05 20:25 Adhemerval Zanella
  2017-09-05 20:25 ` [PATCH 9/9] posix: Fix glob with GLOB_NOCHECK returning modified patterns (BZ#10246) Adhemerval Zanella
                   ` (8 more replies)
  0 siblings, 9 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-05 20:25 UTC (permalink / raw)
  To: libc-alpha; +Cc: Paul Eggert

This patchset sync GLIBC glob with latest gnulib code (064df0b).
Instead of dump a large patchset, I decided to split it since
different commits fixes different GLIBC bugs:

  * Patch 1/9 sync with gnulib code b5ec983 and fixes a GLIBC
    specific code regarding DT_UNKNOWN, DT_DIR, and DT_LNK
    definition.  Paul I think we should sync it back to gnulib.

  * Patch 2/9 sync with gnulib 2adc376c55194 and fixes BZ #19971.

  * Patch 3/9 sync with gnulib fd1daf4 and also fixes BZ #866.

  * Patch 4/9 sync with gnulib 3866ef6.  It is related to
    scratch_buffer code.

  * Patch 5/9 sync with gnulib 5db9301.

  * Patch 6/9 sync with gnulib 064df0b and thus it fixes
    BZ #1062.

  * Patch 7/9 is a glob consolidation to avoid the multiple
    architecture implementations.

  * Patch 8/9 just refactor internal code to use an enumeration
    instead of numeric constants.

  * Patch 9/9 fixes BZ #10246.

I think patches from 1 to 8 should be safe to push.

Adhemerval Zanella (9):
  posix: Sync glob with gnulib [BZ #1062]
  posix: accept inode 0 is a valid inode number (BZ #19971)
  posix: Allow glob to match dangling symlinks [BZ #866]
  Sync scratch_buffer with gnulib
  posix: Fix getpwnam_r usage (BZ #1062)
  posix: fix glob bugs with long login names
  posix: Consolidate glob implementation
  posix: Use enum for __glob_pattern_type result
  posix: Fix glob with GLOB_NOCHECK returning modified patterns
    (BZ#10246)

 ChangeLog                                          | 137 +++
 include/scratch_buffer.h                           |   3 +-
 malloc/scratch_buffer_grow.c                       |   6 +-
 malloc/scratch_buffer_grow_preserve.c              |   6 +-
 malloc/scratch_buffer_set_array_size.c             |   6 +-
 posix/Makefile                                     |   6 +-
 posix/bug-glob1.c                                  |  88 --
 posix/flexmember.h                                 |  45 +
 posix/glob.c                                       | 994 ++++++++-------------
 posix/glob64.c                                     |   6 -
 posix/glob_internal.h                              |  65 ++
 posix/glob_pattern_p.c                             |  33 +
 posix/globfree.c                                   |  41 +
 posix/globfree64.c                                 |  31 +
 posix/globtest.sh                                  |  37 +
 posix/tst-glob_symlinks.c                          | 132 +++
 sysdeps/gnu/glob64.c                               |   3 -
 sysdeps/unix/sysv/linux/Makefile                   |   2 +-
 sysdeps/unix/sysv/linux/alpha/glob.c               |   4 -
 sysdeps/unix/sysv/linux/alpha/globfree.c           |  37 +
 sysdeps/unix/sysv/linux/arm/glob64.c               |   1 -
 sysdeps/unix/sysv/linux/glob.c                     |  28 +
 sysdeps/unix/sysv/linux/glob64.c                   |  51 ++
 sysdeps/unix/sysv/linux/globfree.c                 |  30 +
 sysdeps/unix/sysv/linux/globfree64.c               |  36 +
 sysdeps/unix/sysv/linux/i386/alphasort64.c         |   2 +-
 sysdeps/unix/sysv/linux/i386/getdents64.c          |   2 +-
 sysdeps/unix/sysv/linux/i386/glob64.c              |  79 --
 sysdeps/unix/sysv/linux/i386/readdir64.c           |   2 +-
 sysdeps/unix/sysv/linux/i386/readdir64_r.c         |   2 +-
 sysdeps/unix/sysv/linux/i386/versionsort64.c       |   2 +-
 sysdeps/unix/sysv/linux/m68k/glob64.c              |   1 -
 sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c   |   1 -
 sysdeps/unix/sysv/linux/{i386 => }/olddirent.h     |   0
 sysdeps/unix/sysv/linux/oldglob.c                  |  43 +
 sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c |   1 -
 sysdeps/unix/sysv/linux/s390/s390-32/glob64.c      |   2 +
 sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c     |   1 -
 sysdeps/unix/sysv/linux/wordsize-64/glob64.c       |   2 -
 sysdeps/unix/sysv/linux/x86_64/x32/glob.c          |   1 -
 sysdeps/wordsize-64/glob64.c                       |   1 -
 sysdeps/wordsize-64/{glob.c => globfree.c}         |   5 +-
 42 files changed, 1139 insertions(+), 836 deletions(-)
 delete mode 100644 posix/bug-glob1.c
 create mode 100644 posix/flexmember.h
 create mode 100644 posix/glob_internal.h
 create mode 100644 posix/glob_pattern_p.c
 create mode 100644 posix/globfree.c
 create mode 100644 posix/globfree64.c
 create mode 100644 posix/tst-glob_symlinks.c
 create mode 100644 sysdeps/unix/sysv/linux/alpha/globfree.c
 delete mode 100644 sysdeps/unix/sysv/linux/arm/glob64.c
 create mode 100644 sysdeps/unix/sysv/linux/glob.c
 create mode 100644 sysdeps/unix/sysv/linux/glob64.c
 create mode 100644 sysdeps/unix/sysv/linux/globfree.c
 create mode 100644 sysdeps/unix/sysv/linux/globfree64.c
 delete mode 100644 sysdeps/unix/sysv/linux/i386/glob64.c
 delete mode 100644 sysdeps/unix/sysv/linux/m68k/glob64.c
 delete mode 100644 sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c
 rename sysdeps/unix/sysv/linux/{i386 => }/olddirent.h (100%)
 create mode 100644 sysdeps/unix/sysv/linux/oldglob.c
 delete mode 100644 sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c
 create mode 100644 sysdeps/unix/sysv/linux/s390/s390-32/glob64.c
 delete mode 100644 sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c
 delete mode 100644 sysdeps/unix/sysv/linux/wordsize-64/glob64.c
 delete mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/glob.c
 delete mode 100644 sysdeps/wordsize-64/glob64.c
 rename sysdeps/wordsize-64/{glob.c => globfree.c} (57%)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH 7/9] posix: Consolidate glob implementation
  2017-09-05 20:25 [PATCH 0/9] posix: glob fixes and refactor Adhemerval Zanella
                   ` (3 preceding siblings ...)
  2017-09-05 20:25 ` [PATCH 1/9] posix: Sync glob with gnulib [BZ #1062] Adhemerval Zanella
@ 2017-09-05 20:25 ` Adhemerval Zanella
  2017-09-12  7:35   ` Andreas Schwab
  2017-09-12 12:56   ` Andreas Schwab
  2017-09-05 20:25 ` [PATCH 2/9] posix: accept inode 0 is a valid inode number (BZ #19971) Adhemerval Zanella
                   ` (3 subsequent siblings)
  8 siblings, 2 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-05 20:25 UTC (permalink / raw)
  To: libc-alpha; +Cc: Paul Eggert

This patch consolidates the glob implementation.  The main changes are:

  * On Linux all implementation now uses a default one with the exception
    of alpha (which requires specific versioning) and s390-32 (which
    different than other 32 bits with support for v2.1 symbol does not
    add a compat one).

  * The default implementation uses XSTAT_IS_XSTAT64 to define whether
    both glob{free} and glob{free}64 should be different implementation.
    For archictures that define XSTAT_IS_XSTAT64, glob{free} alias to
    glob{free}64.

  * Move i386 olddirent.h header to Linux default directory, since it is
    the only header with this name and it is shared among different
    architectures (and used on compat glob symbol as well).

Checked on x86_64-linux-gnu and on a build using build-many-glibcs.py
for all major architectures.

	* sysdeps/gnu/globfree64.c: Remove file.
	* sysdeps/unix/sysv/linux/arm/glob64.c: Likewise.
	* sysdeps/unix/sysv/linux/i386/glob64.c: Likewise.
	* sysdeps/unix/sysv/linux/m68k/glob64.c: Likewise.
	* sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c: Likewise.
	* sysdeps/unix/sysv/linux/mips/mips64/n64/globfree64.c: Likewise.
	* sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c: Likewise.
	* sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c: Likewise.
	* sysdeps/unix/sysv/linux/wordsize-64/glob64.c: Likewise.
	* sysdeps/unix/sysv/linux/wordsize-64/globfree64.c: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/x32/glob.c: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/x32/globfree.c: Likewise.
	* sysdeps/wordsize-64/glob.c: Likewise.
	* sysdeps/wordsize-64/glob64.c: Likewise.
	* sysdeps/wordsize-64/globfree64.c: Likewise.
	* sysdeps/unix/sysv/linux/glob.c: New file.
	* sysdeps/unix/sysv/linux/glob64.c: Likewise.
	* sysdeps/unix/sysv/linux/globfree.c: Likewise.
	* sysdeps/unix/sysv/linux/globfree64.c: Likewise.
	* sysdeps/unix/sysv/linux/s390/s390-32/glob64.c: Likewise.
	* sysdeps/unix/sysv/linux/oldglob.c [SHLIB_COMPAT]: Also
	adds !GLOB_NO_OLD_VERSION as an extra condition.
	* sysdeps/unix/sysv/linux/i386/alphasort64.c: Include olddirent.h
	using relative path instead of absolute one.
	* sysdeps/unix/sysv/linux/i386/getdents64.c: Likewise.
	* sysdeps/unix/sysv/linux/i386/readdir64.c: Likewise.
	* sysdeps/unix/sysv/linux/i386/readdir64_r.c: Likewise.
	* sysdeps/unix/sysv/linux/i386/versionsort64.c: Likewise.
	* sysdeps/unix/sysv/linux/i386/olddirent.h: Move to ...
	* sysdeps/unix/sysv/linux//olddirent.h: ... here.
---
 ChangeLog                                          | 31 ++++++++++++++++
 sysdeps/gnu/globfree64.c                           | 10 ------
 sysdeps/unix/sysv/linux/arm/glob64.c               |  1 -
 sysdeps/unix/sysv/linux/glob.c                     | 28 +++++++++++++++
 sysdeps/unix/sysv/linux/{i386 => }/glob64.c        | 41 +++++++++++++---------
 sysdeps/unix/sysv/linux/globfree.c                 | 30 ++++++++++++++++
 sysdeps/unix/sysv/linux/globfree64.c               | 36 +++++++++++++++++++
 sysdeps/unix/sysv/linux/i386/alphasort64.c         |  2 +-
 sysdeps/unix/sysv/linux/i386/getdents64.c          |  2 +-
 sysdeps/unix/sysv/linux/i386/readdir64.c           |  2 +-
 sysdeps/unix/sysv/linux/i386/readdir64_r.c         |  2 +-
 sysdeps/unix/sysv/linux/i386/versionsort64.c       |  2 +-
 sysdeps/unix/sysv/linux/m68k/glob64.c              |  1 -
 sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c   |  1 -
 .../unix/sysv/linux/mips/mips64/n64/globfree64.c   |  1 -
 sysdeps/unix/sysv/linux/{i386 => }/olddirent.h     |  0
 sysdeps/unix/sysv/linux/oldglob.c                  |  5 +--
 sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c |  1 -
 sysdeps/unix/sysv/linux/s390/s390-32/glob64.c      |  2 ++
 sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c     |  1 -
 sysdeps/unix/sysv/linux/wordsize-64/glob64.c       |  2 --
 sysdeps/unix/sysv/linux/wordsize-64/globfree64.c   |  2 --
 sysdeps/unix/sysv/linux/x86_64/x32/glob.c          |  1 -
 sysdeps/unix/sysv/linux/x86_64/x32/globfree.c      |  1 -
 sysdeps/wordsize-64/glob.c                         |  6 ----
 sysdeps/wordsize-64/glob64.c                       |  1 -
 sysdeps/wordsize-64/globfree64.c                   |  1 -
 27 files changed, 160 insertions(+), 53 deletions(-)
 delete mode 100644 sysdeps/gnu/globfree64.c
 delete mode 100644 sysdeps/unix/sysv/linux/arm/glob64.c
 create mode 100644 sysdeps/unix/sysv/linux/glob.c
 rename sysdeps/unix/sysv/linux/{i386 => }/glob64.c (57%)
 create mode 100644 sysdeps/unix/sysv/linux/globfree.c
 create mode 100644 sysdeps/unix/sysv/linux/globfree64.c
 delete mode 100644 sysdeps/unix/sysv/linux/m68k/glob64.c
 delete mode 100644 sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c
 delete mode 100644 sysdeps/unix/sysv/linux/mips/mips64/n64/globfree64.c
 rename sysdeps/unix/sysv/linux/{i386 => }/olddirent.h (100%)
 delete mode 100644 sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c
 create mode 100644 sysdeps/unix/sysv/linux/s390/s390-32/glob64.c
 delete mode 100644 sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c
 delete mode 100644 sysdeps/unix/sysv/linux/wordsize-64/glob64.c
 delete mode 100644 sysdeps/unix/sysv/linux/wordsize-64/globfree64.c
 delete mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/glob.c
 delete mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/globfree.c
 delete mode 100644 sysdeps/wordsize-64/glob.c
 delete mode 100644 sysdeps/wordsize-64/glob64.c
 delete mode 100644 sysdeps/wordsize-64/globfree64.c

diff --git a/sysdeps/gnu/globfree64.c b/sysdeps/gnu/globfree64.c
deleted file mode 100644
index f092d0b..0000000
--- a/sysdeps/gnu/globfree64.c
+++ /dev/null
@@ -1,10 +0,0 @@
-#include <dirent.h>
-#include <glob.h>
-#include <sys/stat.h>
-
-#define glob_t glob64_t
-#define globfree(pglob) globfree64 (pglob)
-
-#include <posix/globfree.c>
-
-libc_hidden_def (globfree64)
diff --git a/sysdeps/unix/sysv/linux/arm/glob64.c b/sysdeps/unix/sysv/linux/arm/glob64.c
deleted file mode 100644
index 82a9a29..0000000
--- a/sysdeps/unix/sysv/linux/arm/glob64.c
+++ /dev/null
@@ -1 +0,0 @@
-#include <sysdeps/unix/sysv/linux/i386/glob64.c>
diff --git a/sysdeps/unix/sysv/linux/glob.c b/sysdeps/unix/sysv/linux/glob.c
new file mode 100644
index 0000000..057ae7f
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/glob.c
@@ -0,0 +1,28 @@
+/* Find pathnames matching a pattern.  Linux version.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <sys/stat.h>
+#include <kernel_stat.h>
+
+#define glob64 __no_glob64_decl
+#include <posix/glob.c>
+#undef glob64
+
+#if XSTAT_IS_XSTAT64
+weak_alias (glob, glob64)
+#endif
diff --git a/sysdeps/unix/sysv/linux/i386/glob64.c b/sysdeps/unix/sysv/linux/glob64.c
similarity index 57%
rename from sysdeps/unix/sysv/linux/i386/glob64.c
rename to sysdeps/unix/sysv/linux/glob64.c
index 230f9fc..428bbac 100644
--- a/sysdeps/unix/sysv/linux/i386/glob64.c
+++ b/sysdeps/unix/sysv/linux/glob64.c
@@ -1,5 +1,5 @@
-/* Two glob variants with 64-bit support, for dirent64 and __olddirent64.
-   Copyright (C) 1998-2017 Free Software Foundation, Inc.
+/* Find pathnames matching a pattern.  Linux version.
+   Copyright (C) 2017 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -16,27 +16,36 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <dirent.h>
-#include <glob.h>
 #include <sys/stat.h>
-#include <shlib-compat.h>
+#include <kernel_stat.h>
 
-#define dirent dirent64
-#define __readdir(dirp) __readdir64 (dirp)
+#if !XSTAT_IS_XSTAT64
+# include <glob.h>
+# include <dirent.h>
+# include <sys/stat.h>
 
-#define glob_t glob64_t
-#define glob(pattern, flags, errfunc, pglob) \
+# define dirent dirent64
+# define __readdir(dirp) __readdir64 (dirp)
+
+# define glob_t glob64_t
+# define glob(pattern, flags, errfunc, pglob) \
   __glob64 (pattern, flags, errfunc, pglob)
-#define globfree(pglob) globfree64 (pglob)
+# define globfree(pglob) globfree64 (pglob)
+
+# undef stat
+# define stat stat64
 
-#undef stat
-#define stat stat64
-#undef __stat
-#define __stat(file, buf) __xstat64 (_STAT_VER, file, buf)
+# define COMPILE_GLOB64	1
 
-#define COMPILE_GLOB64	1
+# include <posix/glob.c>
 
-#include <posix/glob.c>
+# include "shlib-compat.h"
 
+# ifdef GLOB_NO_OLD_VERSION
+strong_alias (__glob64, glob64)
+libc_hidden_def (glob64)
+# else
 versioned_symbol (libc, __glob64, glob64, GLIBC_2_2);
 libc_hidden_ver (__glob64, glob64)
+# endif
+#endif /* XSTAT_IS_XSTAT64  */
diff --git a/sysdeps/unix/sysv/linux/globfree.c b/sysdeps/unix/sysv/linux/globfree.c
new file mode 100644
index 0000000..48d4aec
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/globfree.c
@@ -0,0 +1,30 @@
+/* Frees the dynamically allocated storage from an earlier call to glob.
+   Linux version.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <sys/stat.h>
+#include <kernel_stat.h>
+
+#define globfree64 __no_globfree64_decl
+#include <posix/globfree.c>
+#undef globfree64
+
+#if XSTAT_IS_XSTAT64
+weak_alias (globfree, globfree64)
+libc_hidden_ver (globfree, globfree64)
+#endif
diff --git a/sysdeps/unix/sysv/linux/globfree64.c b/sysdeps/unix/sysv/linux/globfree64.c
new file mode 100644
index 0000000..0020466
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/globfree64.c
@@ -0,0 +1,36 @@
+/* Frees the dynamically allocated storage from an earlier call to glob.
+   Linux version.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <sys/stat.h>
+#include <kernel_stat.h>
+
+#if !XSTAT_IS_XSTAT64
+
+# include <glob.h>
+
+# define glob_t glob64_t
+# define globfree(pglob) globfree64 (pglob)
+
+# undef stat
+# define stat stat64
+
+# include <posix/globfree.c>
+
+libc_hidden_def (globfree64)
+#endif
diff --git a/sysdeps/unix/sysv/linux/i386/alphasort64.c b/sysdeps/unix/sysv/linux/i386/alphasort64.c
index d5fd47a..04b29b6 100644
--- a/sysdeps/unix/sysv/linux/i386/alphasort64.c
+++ b/sysdeps/unix/sysv/linux/i386/alphasort64.c
@@ -30,7 +30,7 @@ versioned_symbol (libc, __alphasort64, alphasort64, GLIBC_2_2);
 
 #if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
 
-#include <sysdeps/unix/sysv/linux/i386/olddirent.h>
+#include <olddirent.h>
 
 int
 __old_alphasort64 (const struct __old_dirent64 **a,
diff --git a/sysdeps/unix/sysv/linux/i386/getdents64.c b/sysdeps/unix/sysv/linux/i386/getdents64.c
index e8b257f..2010bbf 100644
--- a/sysdeps/unix/sysv/linux/i386/getdents64.c
+++ b/sysdeps/unix/sysv/linux/i386/getdents64.c
@@ -28,7 +28,7 @@
 
 #if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
 
-#include <sysdeps/unix/sysv/linux/i386/olddirent.h>
+#include <olddirent.h>
 
 #define __GETDENTS __old_getdents64
 #define DIRENT_TYPE struct __old_dirent64
diff --git a/sysdeps/unix/sysv/linux/i386/readdir64.c b/sysdeps/unix/sysv/linux/i386/readdir64.c
index de8669f..da3defd 100644
--- a/sysdeps/unix/sysv/linux/i386/readdir64.c
+++ b/sysdeps/unix/sysv/linux/i386/readdir64.c
@@ -31,7 +31,7 @@ versioned_symbol (libc, __readdir64, readdir64, GLIBC_2_2);
 
 #if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
 
-#include <sysdeps/unix/sysv/linux/i386/olddirent.h>
+#include <olddirent.h>
 
 #define __READDIR attribute_compat_text_section __old_readdir64
 #define __GETDENTS __old_getdents64
diff --git a/sysdeps/unix/sysv/linux/i386/readdir64_r.c b/sysdeps/unix/sysv/linux/i386/readdir64_r.c
index 344fd53..8c0262d 100644
--- a/sysdeps/unix/sysv/linux/i386/readdir64_r.c
+++ b/sysdeps/unix/sysv/linux/i386/readdir64_r.c
@@ -31,7 +31,7 @@ versioned_symbol (libc, __readdir64_r, readdir64_r, GLIBC_2_2);
 
 #if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
 
-#include <sysdeps/unix/sysv/linux/i386/olddirent.h>
+#include <olddirent.h>
 
 #define __READDIR_R attribute_compat_text_section __old_readdir64_r
 #define __GETDENTS __old_getdents64
diff --git a/sysdeps/unix/sysv/linux/i386/versionsort64.c b/sysdeps/unix/sysv/linux/i386/versionsort64.c
index 3e1c6ea..87f2f95 100644
--- a/sysdeps/unix/sysv/linux/i386/versionsort64.c
+++ b/sysdeps/unix/sysv/linux/i386/versionsort64.c
@@ -30,7 +30,7 @@ versioned_symbol (libc, __versionsort64, versionsort64, GLIBC_2_2);
 
 #if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
 
-#include <sysdeps/unix/sysv/linux/i386/olddirent.h>
+#include <olddirent.h>
 
 int
 __old_versionsort64 (const struct __old_dirent64 **a,
diff --git a/sysdeps/unix/sysv/linux/m68k/glob64.c b/sysdeps/unix/sysv/linux/m68k/glob64.c
deleted file mode 100644
index 82a9a29..0000000
--- a/sysdeps/unix/sysv/linux/m68k/glob64.c
+++ /dev/null
@@ -1 +0,0 @@
-#include <sysdeps/unix/sysv/linux/i386/glob64.c>
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c b/sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c
deleted file mode 100644
index 33918ea..0000000
--- a/sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c
+++ /dev/null
@@ -1 +0,0 @@
-/* glob64 is in glob.c */
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/globfree64.c b/sysdeps/unix/sysv/linux/mips/mips64/n64/globfree64.c
deleted file mode 100644
index abc35fd..0000000
--- a/sysdeps/unix/sysv/linux/mips/mips64/n64/globfree64.c
+++ /dev/null
@@ -1 +0,0 @@
-/* glob64 is in globfree64.c */
diff --git a/sysdeps/unix/sysv/linux/i386/olddirent.h b/sysdeps/unix/sysv/linux/olddirent.h
similarity index 100%
rename from sysdeps/unix/sysv/linux/i386/olddirent.h
rename to sysdeps/unix/sysv/linux/olddirent.h
diff --git a/sysdeps/unix/sysv/linux/oldglob.c b/sysdeps/unix/sysv/linux/oldglob.c
index 8233e57..5402450 100644
--- a/sysdeps/unix/sysv/linux/oldglob.c
+++ b/sysdeps/unix/sysv/linux/oldglob.c
@@ -1,12 +1,13 @@
 #include <shlib-compat.h>
 
-#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
+#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2) \
+    && !defined(GLOB_NO_OLD_VERSION)
 
 #include <dirent.h>
 #include <glob.h>
 #include <sys/stat.h>
 
-#include <sysdeps/unix/sysv/linux/i386/olddirent.h>
+#include <olddirent.h>
 
 int __old_glob64 (const char *__pattern, int __flags,
 		  int (*__errfunc) (const char *, int),
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c b/sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c
deleted file mode 100644
index 82a9a29..0000000
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c
+++ /dev/null
@@ -1 +0,0 @@
-#include <sysdeps/unix/sysv/linux/i386/glob64.c>
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/glob64.c b/sysdeps/unix/sysv/linux/s390/s390-32/glob64.c
new file mode 100644
index 0000000..d220e22
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/glob64.c
@@ -0,0 +1,2 @@
+#define GLOB_NO_OLD_VERSION
+#include <sysdeps/unix/sysv/linux/glob64.c>
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c b/sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c
deleted file mode 100644
index 82a9a29..0000000
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c
+++ /dev/null
@@ -1 +0,0 @@
-#include <sysdeps/unix/sysv/linux/i386/glob64.c>
diff --git a/sysdeps/unix/sysv/linux/wordsize-64/glob64.c b/sysdeps/unix/sysv/linux/wordsize-64/glob64.c
deleted file mode 100644
index eab7703..0000000
--- a/sysdeps/unix/sysv/linux/wordsize-64/glob64.c
+++ /dev/null
@@ -1,2 +0,0 @@
-/* This file is here so sysdeps/gnu/glob64.c doesn't take precedence.  */
-#include <sysdeps/wordsize-64/glob64.c>
diff --git a/sysdeps/unix/sysv/linux/wordsize-64/globfree64.c b/sysdeps/unix/sysv/linux/wordsize-64/globfree64.c
deleted file mode 100644
index af035e1..0000000
--- a/sysdeps/unix/sysv/linux/wordsize-64/globfree64.c
+++ /dev/null
@@ -1,2 +0,0 @@
-/* This file is here so sysdeps/gnu/glob64.c doesn't take precedence.  */
-#include <sysdeps/wordsize-64/globfree64.c>
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/glob.c b/sysdeps/unix/sysv/linux/x86_64/x32/glob.c
deleted file mode 100644
index e542747..0000000
--- a/sysdeps/unix/sysv/linux/x86_64/x32/glob.c
+++ /dev/null
@@ -1 +0,0 @@
-#include <sysdeps/wordsize-64/glob.c>
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/globfree.c b/sysdeps/unix/sysv/linux/x86_64/x32/globfree.c
deleted file mode 100644
index b76a761..0000000
--- a/sysdeps/unix/sysv/linux/x86_64/x32/globfree.c
+++ /dev/null
@@ -1 +0,0 @@
-#include <sysdeps/wordsize-64/globfree.c>
diff --git a/sysdeps/wordsize-64/glob.c b/sysdeps/wordsize-64/glob.c
deleted file mode 100644
index 954e8d3..0000000
--- a/sysdeps/wordsize-64/glob.c
+++ /dev/null
@@ -1,6 +0,0 @@
-#define glob64 __no_glob64_decl
-#define globfree64 __no_globfree64_decl
-#include <posix/glob.c>
-#undef glob64
-#undef globfree64
-weak_alias (glob, glob64)
diff --git a/sysdeps/wordsize-64/glob64.c b/sysdeps/wordsize-64/glob64.c
deleted file mode 100644
index 33918ea..0000000
--- a/sysdeps/wordsize-64/glob64.c
+++ /dev/null
@@ -1 +0,0 @@
-/* glob64 is in glob.c */
diff --git a/sysdeps/wordsize-64/globfree64.c b/sysdeps/wordsize-64/globfree64.c
deleted file mode 100644
index a0f57ff..0000000
--- a/sysdeps/wordsize-64/globfree64.c
+++ /dev/null
@@ -1 +0,0 @@
-/* globfree64 is in globfree.c */
-- 
2.7.4

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-05 20:25 ` [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866] Adhemerval Zanella
@ 2017-09-06  1:27   ` Paul Eggert
  2017-09-06 12:57     ` Adhemerval Zanella
  2017-09-09  9:50   ` Andreas Schwab
  1 sibling, 1 reply; 62+ messages in thread
From: Paul Eggert @ 2017-09-06  1:27 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha

'git am' complains about this patch because it introduces a line with a space 
before a tab.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 8/9] posix: Use enum for __glob_pattern_type result
  2017-09-05 20:25 ` [PATCH 8/9] posix: Use enum for __glob_pattern_type result Adhemerval Zanella
@ 2017-09-06  1:30   ` Paul Eggert
  2017-09-06  4:18   ` Paul Eggert
  1 sibling, 0 replies; 62+ messages in thread
From: Paul Eggert @ 2017-09-06  1:30 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha

'git am' complained about this patch:

.git/rebase-apply/patch:60: trailing whitespace.
   __GLOB_BRACKET   = 0x4
warning: 1 line adds whitespace errors.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 1/9] posix: Sync glob with gnulib [BZ #1062]
  2017-09-05 20:25 ` [PATCH 1/9] posix: Sync glob with gnulib [BZ #1062] Adhemerval Zanella
@ 2017-09-06  2:01   ` Paul Eggert
  2017-09-06 12:52     ` Adhemerval Zanella
  2017-09-12 14:20   ` Andreas Schwab
  1 sibling, 1 reply; 62+ messages in thread
From: Paul Eggert @ 2017-09-06  2:01 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha

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

Adhemerval Zanella wrote:
> +/* Any distinct values will do here.
> +   Undef any existing macros out of the way.  */
> +#ifndef DT_UNKNOWN
> +# define DT_UNKNOWN 0
> +#endif
> +#ifndef DT_DIR
> +# define DT_DIR 1
> +#endif
> +#ifndef DT_LNK
> +# define DT_LNK 2
> +#endif

The comment here does not match the code, as nothing is being undeffed. Instead 
of this change, how about the following?

#if !defined _LIBC && !defined HAVE_STRUCT_DIRENT_D_TYPE
/* Any distinct values will do here.
    Undef any existing macros out of the way.  */
# undef DT_UNKNOWN
# undef DT_DIR
# undef DT_LNK
# define DT_UNKNOWN 0
# define DT_DIR 1
# define DT_LNK 2
#endif

This makes it more clear to the casual reader that this code is for use only 
outside glibc. Also, it's more robust for hopefully only-hypothetical non-glibc 
platforms that define some DT_ symbols but not others, because it guarantees 
their uniqueness in that case. I installed the attached patch into Gnulib, along 
these lines.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-glob-fix-for-use-in-glibc.patch --]
[-- Type: text/x-patch; name="0001-glob-fix-for-use-in-glibc.patch", Size: 1416 bytes --]

From e73addd2704d4eb68b126210f5b41b428d5d4956 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 5 Sep 2017 18:58:50 -0700
Subject: [PATCH] glob: fix for use in glibc

Problem reported by Adhemerval Zanella in:
https://sourceware.org/ml/libc-alpha/2017-09/msg00213.html
* lib/glob.c (DT_UNKNOWN, DT_DIR, DT_LINK):
Do not redefine if _LIBC.
---
 ChangeLog  | 8 ++++++++
 lib/glob.c | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index 351495b..61e3e8c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2017-09-05  Paul Eggert  <eggert@cs.ucla.edu>
+
+	glob: fix for use in glibc
+	Problem reported by Adhemerval Zanella in:
+	https://sourceware.org/ml/libc-alpha/2017-09/msg00213.html
+	* lib/glob.c (DT_UNKNOWN, DT_DIR, DT_LINK):
+	Do not redefine if _LIBC.
+
 2017-09-02  Paul Eggert  <eggert@cs.ucla.edu>
 
 	glob: fix bugs with long login names
diff --git a/lib/glob.c b/lib/glob.c
index 8eb2b97..ddab535 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -80,7 +80,7 @@ static const char *next_brace_sub (const char *begin, int flags) __THROWNL;
 
 typedef uint_fast8_t dirent_type;
 
-#ifndef HAVE_STRUCT_DIRENT_D_TYPE
+#if !defined _LIBC && !defined HAVE_STRUCT_DIRENT_D_TYPE
 /* Any distinct values will do here.
    Undef any existing macros out of the way.  */
 # undef DT_UNKNOWN
-- 
2.7.4


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 8/9] posix: Use enum for __glob_pattern_type result
  2017-09-05 20:25 ` [PATCH 8/9] posix: Use enum for __glob_pattern_type result Adhemerval Zanella
  2017-09-06  1:30   ` Paul Eggert
@ 2017-09-06  4:18   ` Paul Eggert
  2017-09-06 13:04     ` Adhemerval Zanella
  1 sibling, 1 reply; 62+ messages in thread
From: Paul Eggert @ 2017-09-06  4:18 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha, Gnulib bugs

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

Adhemerval Zanella wrote:
> +enum glob_pattern_type_t
> +{
> +  __GLOB_NONE      = 0x0,
> +  __GLOB_SPECIAL   = 0x1,
> +  __GLOB_BACKSLASH = 0x2,
> +  __GLOB_BRACKET   = 0x4
> +};

The identifier glob_pattern_type_t is not used elsewhere, so let's omit it. This 
makes it clearer that we're merely defining handy names for int constants, as 
opposed to defining a new type.

Also, names like __GLOB_NONE could cause problems when Gnulib is used on non-GNU 
platforms, which might use those names for other purposes. As glob_internal.h is 
not user-visible, let's use ordinary names. I suggest GLOBPAT_NONE, 
GLOBPAT_SPECIAL, etc., as done in the attached patch, which I installed into Gnulib.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-glob-Use-enum-for-__glob_pattern_type-result.patch --]
[-- Type: text/x-patch; name="0001-glob-Use-enum-for-__glob_pattern_type-result.patch", Size: 4657 bytes --]

From 36102f8d365655b5d9693ccff0349acc73c60433 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Tue, 5 Sep 2017 21:14:51 -0700
Subject: [PATCH] glob: Use enum for __glob_pattern_type result

From a patch proposed by Adhemerval Zanella in:
https://sourceware.org/ml/libc-alpha/2017-09/msg00212.html
* lib/glob_internal.h (GLOBPAT_NONE, GLOBPAT_SPECIAL)
(GLOBPAT_BACKSLASH, GLOBPAT_BRACKET): New constants.
* lib/glob_internal.h (__glob_pattern_type):
* lib/glob.c (glob):
* lib/glob_pattern_p.c (__glob_pattern_p):
Use them.
---
 ChangeLog            | 10 ++++++++++
 lib/glob.c           |  8 ++++----
 lib/glob_internal.h  | 18 +++++++++++++-----
 lib/glob_pattern_p.c |  2 +-
 4 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 61e3e8c..448bad2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,15 @@
 2017-09-05  Paul Eggert  <eggert@cs.ucla.edu>
 
+	glob: Use enum for __glob_pattern_type result
+	From a patch proposed by Adhemerval Zanella in:
+	https://sourceware.org/ml/libc-alpha/2017-09/msg00212.html
+	* lib/glob_internal.h (GLOBPAT_NONE, GLOBPAT_SPECIAL)
+	(GLOBPAT_BACKSLASH, GLOBPAT_BRACKET): New constants.
+	* lib/glob_internal.h (__glob_pattern_type):
+	* lib/glob.c (glob):
+	* lib/glob_pattern_p.c (__glob_pattern_p):
+	Use them.
+
 	glob: fix for use in glibc
 	Problem reported by Adhemerval Zanella in:
 	https://sourceware.org/ml/libc-alpha/2017-09/msg00213.html
diff --git a/lib/glob.c b/lib/glob.c
index ddab535..4c6c31b 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -903,7 +903,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
      [ which we handle the same, using fnmatch.  Broken unterminated
      pattern bracket expressions ought to be rare enough that it is
      not worth special casing them, fnmatch will do the right thing.  */
-  if (meta & 5)
+  if (meta & (GLOBPAT_SPECIAL | GLOBPAT_BRACKET))
     {
       /* The directory name contains metacharacters, so we
          have to glob for the directory, and then glob for
@@ -1044,7 +1044,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
       size_t old_pathc = pglob->gl_pathc;
       int orig_flags = flags;
 
-      if (meta & 2)
+      if (meta & GLOBPAT_BACKSLASH)
         {
           char *p = strchr (dirname, '\\'), *q;
           /* We need to unescape the dirname string.  It is certainly
@@ -1242,14 +1242,14 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
                        / sizeof init_names->name[0]);
 
   meta = __glob_pattern_type (pattern, !(flags & GLOB_NOESCAPE));
-  if (meta == 0 && (flags & (GLOB_NOCHECK|GLOB_NOMAGIC)))
+  if (meta == GLOBPAT_NONE && (flags & (GLOB_NOCHECK|GLOB_NOMAGIC)))
     {
       /* We need not do any tests.  The PATTERN contains no meta
          characters and we must not return an error therefore the
          result will always contain exactly one name.  */
       flags |= GLOB_NOCHECK;
     }
-  else if (meta == 0)
+  else if (meta == GLOBPAT_NONE)
     {
       union
       {
diff --git a/lib/glob_internal.h b/lib/glob_internal.h
index 12c93660..d118b35 100644
--- a/lib/glob_internal.h
+++ b/lib/glob_internal.h
@@ -19,35 +19,43 @@
 #ifndef GLOB_INTERNAL_H
 # define GLOB_INTERNAL_H
 
+enum
+{
+  GLOBPAT_NONE      = 0x0,
+  GLOBPAT_SPECIAL   = 0x1,
+  GLOBPAT_BACKSLASH = 0x2,
+  GLOBPAT_BRACKET   = 0x4
+};
+
 static inline int
 __glob_pattern_type (const char *pattern, int quote)
 {
   const char *p;
-  int ret = 0;
+  int ret = GLOBPAT_NONE;
 
   for (p = pattern; *p != '\0'; ++p)
     switch (*p)
       {
       case '?':
       case '*':
-        return 1;
+        return GLOBPAT_SPECIAL;
 
       case '\\':
         if (quote)
           {
             if (p[1] != '\0')
               ++p;
-            ret |= 2;
+            ret |= GLOBPAT_BACKSLASH;
           }
         break;
 
       case '[':
-        ret |= 4;
+        ret |= GLOBPAT_BRACKET;
         break;
 
       case ']':
         if (ret & 4)
-          return 1;
+          return GLOBPAT_SPECIAL;
         break;
       }
 
diff --git a/lib/glob_pattern_p.c b/lib/glob_pattern_p.c
index a17d337..8489106 100644
--- a/lib/glob_pattern_p.c
+++ b/lib/glob_pattern_p.c
@@ -28,6 +28,6 @@
 int
 __glob_pattern_p (const char *pattern, int quote)
 {
-  return __glob_pattern_type (pattern, quote) == 1;
+  return __glob_pattern_type (pattern, quote) == GLOBPAT_SPECIAL;
 }
 weak_alias (__glob_pattern_p, glob_pattern_p)
-- 
2.7.4


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 1/9] posix: Sync glob with gnulib [BZ #1062]
  2017-09-06  2:01   ` Paul Eggert
@ 2017-09-06 12:52     ` Adhemerval Zanella
  0 siblings, 0 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-06 12:52 UTC (permalink / raw)
  To: Paul Eggert, libc-alpha



On 05/09/2017 23:01, Paul Eggert wrote:
> Adhemerval Zanella wrote:
>> +/* Any distinct values will do here.
>> +   Undef any existing macros out of the way.  */
>> +#ifndef DT_UNKNOWN
>> +# define DT_UNKNOWN 0
>> +#endif
>> +#ifndef DT_DIR
>> +# define DT_DIR 1
>> +#endif
>> +#ifndef DT_LNK
>> +# define DT_LNK 2
>> +#endif
> 
> The comment here does not match the code, as nothing is being undeffed. Instead of this change, how about the following?

Indeed, I forgot to update the comments after the fix.

> #if !defined _LIBC && !defined HAVE_STRUCT_DIRENT_D_TYPE
> /* Any distinct values will do here.
>    Undef any existing macros out of the way.  */
> # undef DT_UNKNOWN
> # undef DT_DIR
> # undef DT_LNK
> # define DT_UNKNOWN 0
> # define DT_DIR 1
> # define DT_LNK 2
> #endif
> 
> This makes it more clear to the casual reader that this code is for use only outside glibc. Also, it's more robust for hopefully only-hypothetical non-glibc platforms that define some DT_ symbols but not others, because it guarantees their uniqueness in that case. I installed the attached patch into Gnulib, along these lines.

I do not have a strong preference here, I am ok with your approach and I will
update my patch with this change. In fact I would prefer that all this 
boilerplate that is non required for glibc code to be on a specific header
(maybe glob-gnulib.h or something), but we can think about it later. 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-06  1:27   ` Paul Eggert
@ 2017-09-06 12:57     ` Adhemerval Zanella
  0 siblings, 0 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-06 12:57 UTC (permalink / raw)
  To: Paul Eggert, libc-alpha

Thanks I fixed it in my repo.

On 05/09/2017 22:27, Paul Eggert wrote:
> 'git am' complains about this patch because it introduces a line with a space before a tab.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 8/9] posix: Use enum for __glob_pattern_type result
  2017-09-06  4:18   ` Paul Eggert
@ 2017-09-06 13:04     ` Adhemerval Zanella
  2017-09-06 16:18       ` Paul Eggert
  0 siblings, 1 reply; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-06 13:04 UTC (permalink / raw)
  To: Paul Eggert, libc-alpha, Gnulib bugs



On 06/09/2017 01:18, Paul Eggert wrote:
> Adhemerval Zanella wrote:
>> +enum glob_pattern_type_t
>> +{
>> +  __GLOB_NONE      = 0x0,
>> +  __GLOB_SPECIAL   = 0x1,
>> +  __GLOB_BACKSLASH = 0x2,
>> +  __GLOB_BRACKET   = 0x4
>> +};
> 
> The identifier glob_pattern_type_t is not used elsewhere, so let's omit it. This makes it clearer that we're merely defining handy names for int constants, as opposed to defining a new type.

Ack.

> 
> Also, names like __GLOB_NONE could cause problems when Gnulib is used on non-GNU platforms, which might use those names for other purposes. As glob_internal.h is not user-visible, let's use ordinary names. I suggest GLOBPAT_NONE, GLOBPAT_SPECIAL, etc., as done in the attached patch, which I installed into Gnulib.

My understanding was double underscore identifiers are reserved for implementation
(C99 7.1.3 Reserved identifiers).  But I do not have a strong opinion here, I am 
ok with your approach.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 8/9] posix: Use enum for __glob_pattern_type result
  2017-09-06 13:04     ` Adhemerval Zanella
@ 2017-09-06 16:18       ` Paul Eggert
  2017-09-06 16:54         ` Adhemerval Zanella
  0 siblings, 1 reply; 62+ messages in thread
From: Paul Eggert @ 2017-09-06 16:18 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha, Gnulib bugs

Adhemerval Zanella wrote:
> My understanding was double underscore identifiers are reserved for implementation
> (C99 7.1.3 Reserved identifiers).

Yes, and that's the point. When this code is used as part of Gnulib, it is used 
within an application, so any identifiers it uses that start with __ might 
collide with the implementation, which means it's safer to avoid them when it's 
easy, as is the case here.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 8/9] posix: Use enum for __glob_pattern_type result
  2017-09-06 16:18       ` Paul Eggert
@ 2017-09-06 16:54         ` Adhemerval Zanella
  0 siblings, 0 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-06 16:54 UTC (permalink / raw)
  To: Paul Eggert, libc-alpha, Gnulib bugs



On 06/09/2017 13:18, Paul Eggert wrote:
> Adhemerval Zanella wrote:
>> My understanding was double underscore identifiers are reserved for implementation
>> (C99 7.1.3 Reserved identifiers).
> 
> Yes, and that's the point. When this code is used as part of Gnulib, it is used within an application, so any identifiers it uses that start with __ might collide with the implementation, which means it's safer to avoid them when it's easy, as is the case here.

Right, I got your point.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 9/9] posix: Fix glob with GLOB_NOCHECK returning modified patterns (BZ#10246)
  2017-09-05 20:25 ` [PATCH 9/9] posix: Fix glob with GLOB_NOCHECK returning modified patterns (BZ#10246) Adhemerval Zanella
@ 2017-09-07 22:14   ` Paul Eggert
  2017-09-08  9:16     ` Adhemerval Zanella
  0 siblings, 1 reply; 62+ messages in thread
From: Paul Eggert @ 2017-09-07 22:14 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha

Although this is a definite bug and the patch fixes this instance of it, I'm 
afraid other instances remain unfixed. For example:

    glob_t g; glob ("//a*b", 0, NULL, &g)

can do the wrong thing, since glob calls opendir on "/" instead of "//", and on 
some platforms "/" and "//" are different directories (POSIX allows this as a 
special exception).

A more serious example. If you do this:

   ln -s /no-such-file globlink1
   ln -s . globlink2

then:

       glob_t g;
       int res = glob ("globlink[12]/", 0, NULL, &g);
       assert (res == 0 && g.gl_pathc == 1);
       assert (strcmp (g.gl_pathv[0], "globlink2/") == 0);

fails, since glob gets confused about directories and slashes and mistakenly 
returns two results. Although this bug is seemingly unrelated, the underlying 
cause is the same: glob gets confused about whether to include or exclude 
slashes when doing its tests.

I'll take a look at it, though the fix won't be trivial.

PS. This finishes my review of this patchset. Patches 1-8 are OK to be 
installed, with the trivial changes I suggested earlier. This patch (patch 9) 
I'd like to hold off on, until we've had a chance to work out a 
more-comprehensive fix.

PPS. I'm still slowly wending my way through your original patchset. Most 
recently I looked at "[PATCH 07/18] posix: User LOGIN_NAME_MAX for all user 
names in glob" <https://sourceware.org/ml/libc-alpha/2017-08/msg00447.html>. I'm 
afraid a good fix needs to be hairier there too, as POSIX does not require 
LOGIN_NAME_MAX to be suitable for a stack-based buffer, or even to be defined. I 
have a partly-drafted patch which I hope to finish in the not-too-distant future.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 9/9] posix: Fix glob with GLOB_NOCHECK returning modified patterns (BZ#10246)
  2017-09-07 22:14   ` Paul Eggert
@ 2017-09-08  9:16     ` Adhemerval Zanella
  0 siblings, 0 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-08  9:16 UTC (permalink / raw)
  To: Paul Eggert, libc-alpha



On 08/09/2017 00:14, Paul Eggert wrote:
> Although this is a definite bug and the patch fixes this instance of
> it, I'm afraid other instances remain unfixed. For example:
>
>    glob_t g; glob ("//a*b", 0, NULL, &g)
>
> can do the wrong thing, since glob calls opendir on "/" instead of
> "//", and on some platforms "/" and "//" are different directories
> (POSIX allows this as a special exception).
>
> A more serious example. If you do this:
>
>   ln -s /no-such-file globlink1
>   ln -s . globlink2
>
> then:
>
>       glob_t g;
>       int res = glob ("globlink[12]/", 0, NULL, &g);
>       assert (res == 0 && g.gl_pathc == 1);
>       assert (strcmp (g.gl_pathv[0], "globlink2/") == 0);
>
> fails, since glob gets confused about directories and slashes and
> mistakenly returns two results. Although this bug is seemingly
> unrelated, the underlying cause is the same: glob gets confused about
> whether to include or exclude slashes when doing its tests.
>
> I'll take a look at it, though the fix won't be trivial.
>
> PS. This finishes my review of this patchset. Patches 1-8 are OK to be
> installed, with the trivial changes I suggested earlier. This patch
> (patch 9) I'd like to hold off on, until we've had a chance to work
> out a more-comprehensive fix.
Fair enough, I will hold patch 9 push and take a look at the examples you
brought up.  Thanks for the follow up.

>
> PPS. I'm still slowly wending my way through your original patchset.
> Most recently I looked at "[PATCH 07/18] posix: User LOGIN_NAME_MAX
> for all user names in glob"
> <https://sourceware.org/ml/libc-alpha/2017-08/msg00447.html>. I'm
> afraid a good fix needs to be hairier there too, as POSIX does not
> require LOGIN_NAME_MAX to be suitable for a stack-based buffer, or
> even to be defined. I have a partly-drafted patch which I hope to
> finish in the not-too-distant future.
Alright, my initial patch was to adequate it to glibc code (which does
define a actual limit suitable to stack allocation) and get rid of any
alloca usage.  My understanding, based on gnulib commit 064df0b0c,
is it should not impose a limit on user name length. 

So currently in a patchset I am intended to send after this one, user
name handling is now based on my char_array struct and thus allocates
the user_name dinamically if required.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-05 20:25 ` [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866] Adhemerval Zanella
  2017-09-06  1:27   ` Paul Eggert
@ 2017-09-09  9:50   ` Andreas Schwab
  2017-09-09 11:56     ` Adhemerval Zanella
  1 sibling, 1 reply; 62+ messages in thread
From: Andreas Schwab @ 2017-09-09  9:50 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Paul Eggert

This breaks make, it doesn't expect that glob calls gl_lstat.

dir_setup_glob:

  /* We don't bother setting gl_lstat, since glob never calls it.
     The slot is only there for compatibility with 4.4 BSD.  */

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-09  9:50   ` Andreas Schwab
@ 2017-09-09 11:56     ` Adhemerval Zanella
  2017-09-09 17:02       ` Paul Eggert
  0 siblings, 1 reply; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-09 11:56 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: libc-alpha, Paul Eggert

I give you that we should properly document what GLOB_ALTDIRFUNC
expects for partially initialized glob_t alternative functions, but currenyl
glob code assumes that if GLOB_ALTDIRFUNC is set then glob_t
function pointers actually points to valid implementation.

So I think this is essentially a make issue. And since make also packs
its own glob copy from gnulib, it is matter to fix on make if and when
it syncs with gnulib.

On 09/09/2017 11:50, Andreas Schwab wrote:
> This breaks make, it doesn't expect that glob calls gl_lstat.
>
> dir_setup_glob:
>
>   /* We don't bother setting gl_lstat, since glob never calls it.
>      The slot is only there for compatibility with 4.4 BSD.  */
>
> Andreas.
>


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-09 11:56     ` Adhemerval Zanella
@ 2017-09-09 17:02       ` Paul Eggert
  2017-09-09 17:11         ` Zack Weinberg
  2017-09-10  8:19         ` Adhemerval Zanella
  0 siblings, 2 replies; 62+ messages in thread
From: Paul Eggert @ 2017-09-09 17:02 UTC (permalink / raw)
  To: Adhemerval Zanella, Andreas Schwab; +Cc: libc-alpha

Adhemerval Zanella wrote:
> since make also packs
> its own glob copy from gnulib, it is matter to fix on make if and when
> it syncs with gnulib.

No, GNU Make uses glibc glob if it passes the compatibility tests in 
'configure', which it does. So previously-built instances of GNU make will 
likely crash if run with a glibc containing the proposed symlink changes. Even 
if you rebuild GNU Make from scratch it will still crash, because glibc glob 
will pass GNU Make's tests even with the patch.

We could fix this by incrementing _GNU_GLOB_INTERFACE_VERSION to 2 (causing GNU 
Make's configure-time test to fail), but this is a serious step that requires 
changing the libc.so major version number, creating backwards-compatibility 
functions for the old behavior, etc. I doubt whether the symlink glitch with 
'glob' is worth all this effort.

How about the following idea instead: establish two new flags GLOB_FOLLOW and 
GLOB_NOFOLLOW, where the caller specifies whether symlinks should be followed. 
The default is system-dependent. For glibc the default is GLOB_FOLLOW (we can 
even make GLOB_FOLLOW zero). For FreeBSD the default would be GLOB_NOFOLLOW, 
assuming they like the idea of supporting these flags. This maintains 
backward-compatibility for both kinds of platforms. For application code 
preferring GLOB_NOFOLLOW semantics if available, a simple:

#include <glob.h>
#ifndef GLOB_NOFOLLOW
# define GLOB_NOFOLLOW 0
#endif

will do, as long as all calls go glob specify 'GLOB_NOFOLLOW'. We can implement 
this idea first in Gnulib and then propose it for glibc.

Anyway, I'll submit a bug report to GNU Make, since it should not be assuming 
this implementation detail of glibc, regardless of what we decide about the 
above matter. However, it will be at best many years before we can assume this 
bug is fixed in the wild.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-09 17:02       ` Paul Eggert
@ 2017-09-09 17:11         ` Zack Weinberg
  2017-09-09 17:26           ` Andreas Schwab
  2017-09-10  8:19         ` Adhemerval Zanella
  1 sibling, 1 reply; 62+ messages in thread
From: Zack Weinberg @ 2017-09-09 17:11 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Adhemerval Zanella, Andreas Schwab, GNU C Library

On Sat, Sep 9, 2017 at 1:01 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> How about the following idea instead: establish two new flags GLOB_FOLLOW
> and GLOB_NOFOLLOW, where the caller specifies whether symlinks should be
> followed. The default is system-dependent. For glibc the default is
> GLOB_FOLLOW (we can even make GLOB_FOLLOW zero). For FreeBSD the default
> would be GLOB_NOFOLLOW, assuming they like the idea of supporting these
> flags. This maintains backward-compatibility for both kinds of platforms.
> For application code preferring GLOB_NOFOLLOW semantics if available, a
> simple:
>
> #include <glob.h>
> #ifndef GLOB_NOFOLLOW
> # define GLOB_NOFOLLOW 0
> #endif
>
> will do, as long as all calls go glob specify 'GLOB_NOFOLLOW'. We can
> implement this idea first in Gnulib and then propose it for glibc.

This also sounds like a lot of complexity.  With the bug in Make, is
gl_lstat garbage or is it NULL?  We could have the glob implementation
check for NULL.

zw

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-09 17:11         ` Zack Weinberg
@ 2017-09-09 17:26           ` Andreas Schwab
  2017-09-09 17:33             ` Zack Weinberg
  0 siblings, 1 reply; 62+ messages in thread
From: Andreas Schwab @ 2017-09-09 17:26 UTC (permalink / raw)
  To: Zack Weinberg; +Cc: Paul Eggert, Adhemerval Zanella, GNU C Library

On Sep 09 2017, Zack Weinberg <zackw@panix.com> wrote:

> This also sounds like a lot of complexity.  With the bug in Make, is
> gl_lstat garbage or is it NULL?

It is uninitialized.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-09 17:26           ` Andreas Schwab
@ 2017-09-09 17:33             ` Zack Weinberg
  0 siblings, 0 replies; 62+ messages in thread
From: Zack Weinberg @ 2017-09-09 17:33 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Paul Eggert, Adhemerval Zanella, GNU C Library

On Sat, Sep 9, 2017 at 1:26 PM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Sep 09 2017, Zack Weinberg <zackw@panix.com> wrote:
>
>> This also sounds like a lot of complexity.  With the bug in Make, is
>> gl_lstat garbage or is it NULL?
>
> It is uninitialized.

Drat.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-09 17:02       ` Paul Eggert
  2017-09-09 17:11         ` Zack Weinberg
@ 2017-09-10  8:19         ` Adhemerval Zanella
  2017-09-10 17:13           ` Paul Eggert
  2017-09-11 14:34           ` Joseph Myers
  1 sibling, 2 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-10  8:19 UTC (permalink / raw)
  To: Paul Eggert, Andreas Schwab; +Cc: libc-alpha



On 09/09/2017 19:01, Paul Eggert wrote:
> Adhemerval Zanella wrote:
>> since make also packs
>> its own glob copy from gnulib, it is matter to fix on make if and when
>> it syncs with gnulib.
>
> No, GNU Make uses glibc glob if it passes the compatibility tests in
> 'configure', which it does. So previously-built instances of GNU make
> will likely crash if run with a glibc containing the proposed symlink
> changes. Even if you rebuild GNU Make from scratch it will still
> crash, because glibc glob will pass GNU Make's tests even with the patch.
>
> We could fix this by incrementing _GNU_GLOB_INTERFACE_VERSION to 2
> (causing GNU Make's configure-time test to fail), but this is a
> serious step that requires changing the libc.so major version number,
> creating backwards-compatibility functions for the old behavior, etc.
> I doubt whether the symlink glitch with 'glob' is worth all this effort.
>
> How about the following idea instead: establish two new flags
> GLOB_FOLLOW and GLOB_NOFOLLOW, where the caller specifies whether
> symlinks should be followed. The default is system-dependent. For
> glibc the default is GLOB_FOLLOW (we can even make GLOB_FOLLOW zero).
> For FreeBSD the default would be GLOB_NOFOLLOW, assuming they like the
> idea of supporting these flags. This maintains backward-compatibility
> for both kinds of platforms. For application code preferring
> GLOB_NOFOLLOW semantics if available, a simple:
>
> #include <glob.h>
> #ifndef GLOB_NOFOLLOW
> # define GLOB_NOFOLLOW 0
> #endif
>
> will do, as long as all calls go glob specify 'GLOB_NOFOLLOW'. We can
> implement this idea first in Gnulib and then propose it for glibc.
>
> Anyway, I'll submit a bug report to GNU Make, since it should not be
> assuming this implementation detail of glibc, regardless of what we
> decide about the above matter. However, it will be at best many years
> before we can assume this bug is fixed in the wild.
I would prefer to avoid adding a new flag, but for this specific issue I
do not see a better
solution (as you have said I also agree bumping interface version does
not worth the
trouble).  What really bothers me is the motivation to actually support
it is to maintain
compatibility with a undefined use of a not well documented interface
(which imho is
clearly a bug in 'make' usage). This will be another adhoc gnu
extension, which
most likely won't be used anywhere besides on make itself (and the
system-dependent
semantic will also lead to more confusion).

Another option is to add a compat glob symbol with previous semantic
(without
actually bumping _GNU_GLOB_INTERFACE_VERSION). It still won't help new
'make'
builds against newer glibc (not without fixing make anyway), so I am not
sure if
is feasible solution.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-10  8:19         ` Adhemerval Zanella
@ 2017-09-10 17:13           ` Paul Eggert
  2017-09-11 14:34           ` Joseph Myers
  1 sibling, 0 replies; 62+ messages in thread
From: Paul Eggert @ 2017-09-10 17:13 UTC (permalink / raw)
  To: Adhemerval Zanella, Andreas Schwab; +Cc: libc-alpha

Adhemerval Zanella wrote:

> I would prefer to avoid adding a new flag,

Me too.

> Another option is to add a compat glob symbol with previous semantic
> (without
> actually bumping _GNU_GLOB_INTERFACE_VERSION).

I like this option better than what I suggested. How about if you propose a 
patch along those lines?

PS. I sent in a bug report for GNU Make about a half hour ago. See:

http://lists.gnu.org/archive/html/bug-make/2017-09/msg00014.html

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-10  8:19         ` Adhemerval Zanella
  2017-09-10 17:13           ` Paul Eggert
@ 2017-09-11 14:34           ` Joseph Myers
  2017-09-11 14:38             ` Zack Weinberg
  1 sibling, 1 reply; 62+ messages in thread
From: Joseph Myers @ 2017-09-11 14:34 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: Paul Eggert, Andreas Schwab, libc-alpha

On Sun, 10 Sep 2017, Adhemerval Zanella wrote:

> Another option is to add a compat glob symbol with previous semantic 
> (without actually bumping _GNU_GLOB_INTERFACE_VERSION). It still won't 

To be clear, that's adding new symbol versions of glob and glob64 
everywhere, making all existing versions (some configurations already have 
more than one version of glob or glob64) ignore gl_lstat for 
compatibility.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-11 14:34           ` Joseph Myers
@ 2017-09-11 14:38             ` Zack Weinberg
  2017-09-11 16:53               ` Paul Eggert
  0 siblings, 1 reply; 62+ messages in thread
From: Zack Weinberg @ 2017-09-11 14:38 UTC (permalink / raw)
  To: Joseph Myers
  Cc: Adhemerval Zanella, Paul Eggert, Andreas Schwab, GNU C Library

On Mon, Sep 11, 2017 at 10:33 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Sun, 10 Sep 2017, Adhemerval Zanella wrote:
>
>> Another option is to add a compat glob symbol with previous semantic
>> (without actually bumping _GNU_GLOB_INTERFACE_VERSION). It still won't
>
> To be clear, that's adding new symbol versions of glob and glob64
> everywhere, making all existing versions (some configurations already have
> more than one version of glob or glob64) ignore gl_lstat for
> compatibility.

If this is done, the new symbol version should attempt to validate the
altdirfuncs structure on entry, so that the buggy versions of make
(and any other programs with the same bug) get a reliable failure
rather than crashing only if glob thinks it needs to call gl_lstat.
Otherwise it'll be like the memcpy mess, where people didn't notice
that they had a bug they needed to fix.

zw

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-11 14:38             ` Zack Weinberg
@ 2017-09-11 16:53               ` Paul Eggert
  2017-09-11 17:25                 ` Zack Weinberg
  0 siblings, 1 reply; 62+ messages in thread
From: Paul Eggert @ 2017-09-11 16:53 UTC (permalink / raw)
  To: Zack Weinberg, Joseph Myers
  Cc: Adhemerval Zanella, Andreas Schwab, GNU C Library

On 09/11/2017 07:38 AM, Zack Weinberg wrote:
> If this is done, the new symbol version should attempt to validate the
> altdirfuncs structure on entry

I don't see how glob could do that reliably and cheaply. How does one 
validate that a pointer to a stat-like function is actually a pointer to 
a stat-like function? All one can do is call the function and pray.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-11 16:53               ` Paul Eggert
@ 2017-09-11 17:25                 ` Zack Weinberg
  2017-09-11 17:38                   ` Paul Eggert
  0 siblings, 1 reply; 62+ messages in thread
From: Zack Weinberg @ 2017-09-11 17:25 UTC (permalink / raw)
  To: Paul Eggert
  Cc: Joseph Myers, Adhemerval Zanella, Andreas Schwab, GNU C Library

On Mon, Sep 11, 2017 at 12:53 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> On 09/11/2017 07:38 AM, Zack Weinberg wrote:
>>
>> If this is done, the new symbol version should attempt to validate the
>> altdirfuncs structure on entry
>
>
> I don't see how glob could do that reliably and cheaply. How does one
> validate that a pointer to a stat-like function is actually a pointer to a
> stat-like function? All one can do is call the function and pray.

It should be enough to make a dummy call, e.g.

 pglob->gl_lstat(".", &statbuf);

zw

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-11 17:25                 ` Zack Weinberg
@ 2017-09-11 17:38                   ` Paul Eggert
  2017-09-11 17:56                     ` Zack Weinberg
  0 siblings, 1 reply; 62+ messages in thread
From: Paul Eggert @ 2017-09-11 17:38 UTC (permalink / raw)
  To: Zack Weinberg
  Cc: Joseph Myers, Adhemerval Zanella, Andreas Schwab, GNU C Library

On 09/11/2017 10:25 AM, Zack Weinberg wrote:
>
> It should be enough to make a dummy call, e.g.
>
>   pglob->gl_lstat(".", &statbuf);
>

Unfortunately calling lstat is quite expensive on some (non-POSIX) 
platforms, even on the working directory. So we can't do the above in 
the Gnulib version. Besides, under the proposed patch glob is going to 
use gl_lstat instead of gl_stat in almost all cases, so the dummy call 
won't add much extra checking.

I suppose we could valid gl_stat instead, as gl_stat usage will become 
rare (used only if GLOB_MARK is also specified, just before returning 
results). But we don't have any code in the wild that is giving us 
invalid gl_stat pointers, so it wouldn't be that helpful to try to 
validate gl_stat either.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-11 17:38                   ` Paul Eggert
@ 2017-09-11 17:56                     ` Zack Weinberg
  2017-09-11 18:03                       ` Paul Eggert
  0 siblings, 1 reply; 62+ messages in thread
From: Zack Weinberg @ 2017-09-11 17:56 UTC (permalink / raw)
  To: Paul Eggert
  Cc: Joseph Myers, Adhemerval Zanella, Andreas Schwab, GNU C Library

On Mon, Sep 11, 2017 at 1:38 PM, Paul Eggert <eggert@cs.ucla.edu> wrote:
> On 09/11/2017 10:25 AM, Zack Weinberg wrote:
>>
>>
>> It should be enough to make a dummy call, e.g.
>>
>>   pglob->gl_lstat(".", &statbuf);
>>
>
> Unfortunately calling lstat is quite expensive on some (non-POSIX)
> platforms, even on the working directory. So we can't do the above in the
> Gnulib version.

I have trouble believing this will be a measurable performance hit,
considering how much other expensive work glob has to do.

> Besides, under the proposed patch glob is going to use
> gl_lstat instead of gl_stat in almost all cases, so the dummy call won't add
> much extra checking.

The point is not to add _extra_ checking; the point is to ensure that
gl_lstat (and gl_stat) are valid on all calls, _even if_ they wouldn't
otherwise have been used.  I'm trying to turn "may fail at runtime
under rare circumstances" into "will definitely fail at runtime on the
first use", which is the best we can do in C.

> I suppose we could valid gl_stat instead, as gl_stat usage will become rare
> (used only if GLOB_MARK is also specified, just before returning results).
> But we don't have any code in the wild that is giving us invalid gl_stat
> pointers, so it wouldn't be that helpful to try to validate gl_stat either.

So here's an alternative, less thorough but perhaps also less costly
approach: when GLOB_ALTDIRFUNCS is set, call both gl_stat and gl_lstat
on the first name that's going to be returned, even if we have no
other reason to do this.  Optionally, memoize the function pointer and
don't bother making the extra call again if we recognize that it's
known to work.

(Maybe also it would be a good idea to check up front for any NULL
callbacks in the ALTDIRFUNCS case.)

zw

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-11 17:56                     ` Zack Weinberg
@ 2017-09-11 18:03                       ` Paul Eggert
  2017-09-11 20:09                         ` Adhemerval Zanella
  0 siblings, 1 reply; 62+ messages in thread
From: Paul Eggert @ 2017-09-11 18:03 UTC (permalink / raw)
  To: Zack Weinberg
  Cc: Joseph Myers, Adhemerval Zanella, Andreas Schwab, GNU C Library

On 09/11/2017 10:56 AM, Zack Weinberg wrote:
> So here's an alternative, less thorough but perhaps also less costly
> approach: when GLOB_ALTDIRFUNCS is set, call both gl_stat and gl_lstat
> on the first name that's going to be returned, even if we have no
> other reason to do this.
Something like that would be better, yes. Still not sure it's worth the 
trouble. We don't know how expensive gl_stat and gl_lstat will be, in 
general.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-11 18:03                       ` Paul Eggert
@ 2017-09-11 20:09                         ` Adhemerval Zanella
  2017-09-13  9:14                           ` Paul Eggert
  0 siblings, 1 reply; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-11 20:09 UTC (permalink / raw)
  To: Paul Eggert, Zack Weinberg; +Cc: Joseph Myers, Andreas Schwab, GNU C Library



On 11/09/2017 15:03, Paul Eggert wrote:
> On 09/11/2017 10:56 AM, Zack Weinberg wrote:
>> So here's an alternative, less thorough but perhaps also less costly
>> approach: when GLOB_ALTDIRFUNCS is set, call both gl_stat and gl_lstat
>> on the first name that's going to be returned, even if we have no
>> other reason to do this.
> Something like that would be better, yes. Still not sure it's worth the trouble. We don't know how expensive gl_stat and gl_lstat will be, in general.

Another approach that does not involve adding compat symbols (which adds
a lot of code complexity inside glibc build and do not solve 'make' builds
against new glibc) would to make GLOB_ALTDIRFUNCS to follow the old semantic
of using gl_stat instead of gl_lstat while making glob without GLOB_ALTDIRFUNCS 
works as intended.  And add another flag, GLOB_ALTDIRFUNCS2, which actually 
uses gl_lstat.

It will solve make compat issue even for build against newer glibcs with
the cost of making GLOB_ALTDIRFUNCS with a slight different semantic than
default glob (which given the current situation is a feasible cost).

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 7/9] posix: Consolidate glob implementation
  2017-09-05 20:25 ` [PATCH 7/9] posix: Consolidate glob implementation Adhemerval Zanella
@ 2017-09-12  7:35   ` Andreas Schwab
  2017-09-12 14:08     ` Adhemerval Zanella
  2017-09-12 14:29     ` Joseph Myers
  2017-09-12 12:56   ` Andreas Schwab
  1 sibling, 2 replies; 62+ messages in thread
From: Andreas Schwab @ 2017-09-12  7:35 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Paul Eggert

On Sep 05 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

>   * On Linux all implementation now uses a default one with the exception
>     of alpha (which requires specific versioning) and s390-32 (which
>     different than other 32 bits with support for v2.1 symbol does not
>     add a compat one).

This drops the default glob64 symbol from s390 (the glob64@GLIBC_2.1
symbol is now hidden).

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 7/9] posix: Consolidate glob implementation
  2017-09-05 20:25 ` [PATCH 7/9] posix: Consolidate glob implementation Adhemerval Zanella
  2017-09-12  7:35   ` Andreas Schwab
@ 2017-09-12 12:56   ` Andreas Schwab
  2017-09-12 14:22     ` Adhemerval Zanella
  1 sibling, 1 reply; 62+ messages in thread
From: Andreas Schwab @ 2017-09-12 12:56 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Paul Eggert

On Sep 05 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> diff --git a/sysdeps/unix/sysv/linux/oldglob.c b/sysdeps/unix/sysv/linux/oldglob.c
> index 8233e57..5402450 100644
> --- a/sysdeps/unix/sysv/linux/oldglob.c
> +++ b/sysdeps/unix/sysv/linux/oldglob.c
> @@ -1,12 +1,13 @@
>  #include <shlib-compat.h>
>  
> -#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
> +#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2) \
> +    && !defined(GLOB_NO_OLD_VERSION)

There is nothing that defines GLOB_NO_OLD_VERSION for s390-32.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 7/9] posix: Consolidate glob implementation
  2017-09-12  7:35   ` Andreas Schwab
@ 2017-09-12 14:08     ` Adhemerval Zanella
  2017-09-12 14:17       ` Andreas Schwab
  2017-09-12 14:29     ` Joseph Myers
  1 sibling, 1 reply; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-12 14:08 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: libc-alpha, Paul Eggert



On 12/09/2017 04:35, Andreas Schwab wrote:
> On Sep 05 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>>   * On Linux all implementation now uses a default one with the exception
>>     of alpha (which requires specific versioning) and s390-32 (which
>>     different than other 32 bits with support for v2.1 symbol does not
>>     add a compat one).
> 
> This drops the default glob64 symbol from s390 (the glob64@GLIBC_2.1
> symbol is now hidden).
> 
> Andreas.
> 

That was not what I am seeing from master:

$ file libc.so
libc.so: ELF 32-bit MSB shared object, IBM S/390, version 1 (GNU/Linux), dynamically linked, interpreter /lib/ld.so.1, for GNU/Linux 3.2.0, not stripped
$ s390x-glibc-linux-gnu-objdump -t libc.so | grep glob64
00000000 l    df *ABS*  00000000              glob64.os
001221c8 l     F .text  0000144e              __GI___old_glob64
000b3330 l     F .text  0000144e              glob64
000b3330 l     F .text  0000144e              __GI_glob64
000b3330 l     F .text  0000144e              __glob64
001221c8 l     F .text  0000144e              __old_glob64
001221c8 g     F .text  0000144e              glob64@GLIBC_2.1

And checking the build of glob64.os for s390 I do see:

[...]
extern __typeof (__glob64) glob64 __attribute__ ((alias ("__glob64")));
extern __typeof (glob64) __EI_glob64 __asm__("" "glob64"); extern __typeof (glob64) __EI_glob64 __attribute__((alias ("" "__GI_glob64")));
[...]

$ s390x-glibc-linux-gnu-objdump -t posix/glob64.os | grep glob64
posix/glob64.os:     file format elf32-s390
000009b0 g     F .text  0000144e __glob64
000009b0 g     F .text  0000144e .hidden __GI_glob64
000009b0 g     F .text  0000144e glob64

And make check/check-abi does not report this issue as well.  That kind of issue
should be reported by make check-abi, shouldn't it? Are you seeing with on 
make check?

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 7/9] posix: Consolidate glob implementation
  2017-09-12 14:08     ` Adhemerval Zanella
@ 2017-09-12 14:17       ` Andreas Schwab
  0 siblings, 0 replies; 62+ messages in thread
From: Andreas Schwab @ 2017-09-12 14:17 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Paul Eggert

On Sep 12 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> That was not what I am seeing from master:

It comes from oldglob.

> And make check/check-abi does not report this issue as well.  That kind of issue
> should be reported by make check-abi, shouldn't it?

No, this is part of the API, not the ABI (which is still correct).

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 1/9] posix: Sync glob with gnulib [BZ #1062]
  2017-09-05 20:25 ` [PATCH 1/9] posix: Sync glob with gnulib [BZ #1062] Adhemerval Zanella
  2017-09-06  2:01   ` Paul Eggert
@ 2017-09-12 14:20   ` Andreas Schwab
  2017-09-12 17:06     ` Adhemerval Zanella
  1 sibling, 1 reply; 62+ messages in thread
From: Andreas Schwab @ 2017-09-12 14:20 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Paul Eggert

On Sep 05 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
> index e571fe2..30bd167 100644
> --- a/sysdeps/unix/sysv/linux/Makefile
> +++ b/sysdeps/unix/sysv/linux/Makefile
> @@ -137,7 +137,7 @@ endif
>  ifeq ($(subdir),posix)
>  sysdep_headers += bits/initspin.h
>  
> -sysdep_routines += sched_getcpu
> +sysdep_routines += sched_getcpu oldglob

This means that oldglob is now added twice on alpha.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 7/9] posix: Consolidate glob implementation
  2017-09-12 12:56   ` Andreas Schwab
@ 2017-09-12 14:22     ` Adhemerval Zanella
  2017-09-12 14:34       ` Andreas Schwab
  0 siblings, 1 reply; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-12 14:22 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: libc-alpha, Paul Eggert



On 12/09/2017 09:56, Andreas Schwab wrote:
> On Sep 05 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>> diff --git a/sysdeps/unix/sysv/linux/oldglob.c b/sysdeps/unix/sysv/linux/oldglob.c
>> index 8233e57..5402450 100644
>> --- a/sysdeps/unix/sysv/linux/oldglob.c
>> +++ b/sysdeps/unix/sysv/linux/oldglob.c
>> @@ -1,12 +1,13 @@
>>  #include <shlib-compat.h>
>>  
>> -#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
>> +#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2) \
>> +    && !defined(GLOB_NO_OLD_VERSION)
> 
> There is nothing that defines GLOB_NO_OLD_VERSION for s390-32.
> 
> Andreas.
> 

Yes, this is wrong since oldglob.c as default will create symbol where
for s390 it should not.  S390 requires a arch-specific oldglob.c as

#define GLOB_NO_OLD_VERSION
#include <sysdeps/unix/sysv/linux/oldglob.c>

So glob64.os will the only object providing glob64 symbols.  With this
file now I see:

$ /s390x-glibc-linux-gnu-objdump -t posix/glob64.os | grep glob64
posix/glob64.os:     file format elf32-s390
000009b0 g     F .text  0000144e __glob64
000009b0 g     F .text  0000144e .hidden __GI_glob64
000009b0 g     F .text  0000144e glob64
$ s390x-glibc-linux-gnu-objdump -t posix/oldglob.os | grep glob64
$ 

I will fix it and maybe it is confusing the linker and making you seem
the missing glob64 (since oldglob.os and glob64.os both provide the
same symbol).

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 7/9] posix: Consolidate glob implementation
  2017-09-12  7:35   ` Andreas Schwab
  2017-09-12 14:08     ` Adhemerval Zanella
@ 2017-09-12 14:29     ` Joseph Myers
  2017-09-12 14:39       ` Andreas Schwab
  1 sibling, 1 reply; 62+ messages in thread
From: Joseph Myers @ 2017-09-12 14:29 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Adhemerval Zanella, libc-alpha, Paul Eggert

On Tue, 12 Sep 2017, Andreas Schwab wrote:

> On Sep 05 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
> >   * On Linux all implementation now uses a default one with the exception
> >     of alpha (which requires specific versioning) and s390-32 (which
> >     different than other 32 bits with support for v2.1 symbol does not
> >     add a compat one).
> 
> This drops the default glob64 symbol from s390 (the glob64@GLIBC_2.1
> symbol is now hidden).

Does this mean we don't have any tests that verify a call to glob64 can be 
linked (and works)?  If so, such tests should be added (generally, all 
non-compat symbols ought to be tested).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 7/9] posix: Consolidate glob implementation
  2017-09-12 14:22     ` Adhemerval Zanella
@ 2017-09-12 14:34       ` Andreas Schwab
  2017-09-13 12:26         ` Adhemerval Zanella
  0 siblings, 1 reply; 62+ messages in thread
From: Andreas Schwab @ 2017-09-12 14:34 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Paul Eggert

On Sep 12 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> I will fix it and maybe it is confusing the linker and making you seem
> the missing glob64 (since oldglob.os and glob64.os both provide the
> same symbol).

glob64@GLIBC_2.1 is the only version and it isn't the default, thus
becomes hidden.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 7/9] posix: Consolidate glob implementation
  2017-09-12 14:29     ` Joseph Myers
@ 2017-09-12 14:39       ` Andreas Schwab
  2017-09-12 14:50         ` Joseph Myers
  0 siblings, 1 reply; 62+ messages in thread
From: Andreas Schwab @ 2017-09-12 14:39 UTC (permalink / raw)
  To: Joseph Myers; +Cc: Adhemerval Zanella, libc-alpha, Paul Eggert

On Sep 12 2017, Joseph Myers <joseph@codesourcery.com> wrote:

> Does this mean we don't have any tests that verify a call to glob64 can be 
> linked (and works)?  If so, such tests should be added (generally, all 
> non-compat symbols ought to be tested).

It's a long standing bug that we don't verify the API.  See the various
incidences about missing symbols on some architectures.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 7/9] posix: Consolidate glob implementation
  2017-09-12 14:39       ` Andreas Schwab
@ 2017-09-12 14:50         ` Joseph Myers
  0 siblings, 0 replies; 62+ messages in thread
From: Joseph Myers @ 2017-09-12 14:50 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Adhemerval Zanella, libc-alpha, Paul Eggert

On Tue, 12 Sep 2017, Andreas Schwab wrote:

> On Sep 12 2017, Joseph Myers <joseph@codesourcery.com> wrote:
> 
> > Does this mean we don't have any tests that verify a call to glob64 can be 
> > linked (and works)?  If so, such tests should be added (generally, all 
> > non-compat symbols ought to be tested).
> 
> It's a long standing bug that we don't verify the API.  See the various
> incidences about missing symbols on some architectures.

Well, I think of it as a large number of bugs that some symbol X is 
missing test coverage.  In general, each of those bugs can be addressed 
independently by adding a test or tests of an untested interface.  See 
<https://sourceware.org/ml/libc-alpha/2013-07/msg00386.html> for some 
scripts I used to produce a list of untested symbols.  Ideally, everything 
in the API would indeed be tested and documented.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 1/9] posix: Sync glob with gnulib [BZ #1062]
  2017-09-12 14:20   ` Andreas Schwab
@ 2017-09-12 17:06     ` Adhemerval Zanella
  0 siblings, 0 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-12 17:06 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: libc-alpha, Paul Eggert



On 12/09/2017 11:20, Andreas Schwab wrote:
> On Sep 05 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>> diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
>> index e571fe2..30bd167 100644
>> --- a/sysdeps/unix/sysv/linux/Makefile
>> +++ b/sysdeps/unix/sysv/linux/Makefile
>> @@ -137,7 +137,7 @@ endif
>>  ifeq ($(subdir),posix)
>>  sysdep_headers += bits/initspin.h
>>  
>> -sysdep_routines += sched_getcpu
>> +sysdep_routines += sched_getcpu oldglob
> 
> This means that oldglob is now added twice on alpha.
> 
> Andreas.
> 

Ack, I will fix it along with s390.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-11 20:09                         ` Adhemerval Zanella
@ 2017-09-13  9:14                           ` Paul Eggert
  2017-09-13 12:22                             ` Adhemerval Zanella
  2017-09-15 20:18                             ` Florian Weimer
  0 siblings, 2 replies; 62+ messages in thread
From: Paul Eggert @ 2017-09-13  9:14 UTC (permalink / raw)
  To: Adhemerval Zanella, Zack Weinberg
  Cc: Joseph Myers, Andreas Schwab, GNU C Library

Adhemerval Zanella wrote:
> Another approach that does not involve adding compat symbols (which adds
> a lot of code complexity inside glibc build and do not solve 'make' builds
> against new glibc) would to make GLOB_ALTDIRFUNCS to follow the old semantic
> of using gl_stat instead of gl_lstat while making glob without GLOB_ALTDIRFUNCS
> works as intended.  And add another flag, GLOB_ALTDIRFUNCS2, which actually
> uses gl_lstat.

Although that's clever, it is a gratuitous source-code incompatibility with BSD, 
which is not a good thing. To some extent it's just GLOB_FOLLOW and 
GLOB_NOFOLLOW in disguise, and disguise is not a good thing in APIs. So I think 
I still prefer the compat symbol approach.

We'll get GNU 'Make' fixed, and I wouldn't worry overly much about people 
building unpatched 'Make' with new glibc. I filed a Make bug report is here:

http://lists.gnu.org/archive/html/bug-make/2017-09/msg00014.html

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-13  9:14                           ` Paul Eggert
@ 2017-09-13 12:22                             ` Adhemerval Zanella
  2017-09-14 10:05                               ` Szabolcs Nagy
  2017-09-15 20:18                             ` Florian Weimer
  1 sibling, 1 reply; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-13 12:22 UTC (permalink / raw)
  To: Paul Eggert, Zack Weinberg; +Cc: Joseph Myers, Andreas Schwab, GNU C Library



On 13/09/2017 06:14, Paul Eggert wrote:
> Adhemerval Zanella wrote:
>> Another approach that does not involve adding compat symbols (which adds
>> a lot of code complexity inside glibc build and do not solve 'make' builds
>> against new glibc) would to make GLOB_ALTDIRFUNCS to follow the old semantic
>> of using gl_stat instead of gl_lstat while making glob without GLOB_ALTDIRFUNCS
>> works as intended.  And add another flag, GLOB_ALTDIRFUNCS2, which actually
>> uses gl_lstat.
> 
> Although that's clever, it is a gratuitous source-code incompatibility with BSD, which is not a good thing. To some extent it's just GLOB_FOLLOW and GLOB_NOFOLLOW in disguise, and disguise is not a good thing in APIs. So I think I still prefer the compat symbol approach.
> 
> We'll get GNU 'Make' fixed, and I wouldn't worry overly much about people building unpatched 'Make' with new glibc. I filed a Make bug report is here:
> 
> http://lists.gnu.org/archive/html/bug-make/2017-09/msg00014.html

Right, I am mainly trying to avoid bring more internal glob implementation
complexity to glibc, but since you says the unpatched 'Make' built
against newer glibc shouldn't be a problem I think we can this way.
I will work on it.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 7/9] posix: Consolidate glob implementation
  2017-09-12 14:34       ` Andreas Schwab
@ 2017-09-13 12:26         ` Adhemerval Zanella
  0 siblings, 0 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-13 12:26 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: libc-alpha, Paul Eggert



On 12/09/2017 11:34, Andreas Schwab wrote:
> On Sep 12 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>> I will fix it and maybe it is confusing the linker and making you seem
>> the missing glob64 (since oldglob.os and glob64.os both provide the
>> same symbol).
> 
> glob64@GLIBC_2.1 is the only version and it isn't the default, thus
> becomes hidden.
> 
> Andreas.
> 

I pushed the patch below. It removes the alpha rule to add the oldglob
and refrain s390 to build the compat code.

---

diff --git a/sysdeps/unix/sysv/linux/alpha/Makefile b/sysdeps/unix/sysv/linux/alpha/Makefile
index 47bd189..50f4fb1 100644
--- a/sysdeps/unix/sysv/linux/alpha/Makefile
+++ b/sysdeps/unix/sysv/linux/alpha/Makefile
@@ -1,7 +1,3 @@
-ifeq ($(subdir),posix)
-sysdep_routines += oldglob
-endif
-
 ifeq ($(subdir),stdlib)
 gen-as-const-headers += ucontext-offsets.sym
 endif
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/oldglob.c b/sysdeps/unix/sysv/linux/s390/s390-32/oldglob.c
new file mode 100644
index 0000000..56d7d12
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/oldglob.c
@@ -0,0 +1,2 @@
+#define GLOB_NO_OLD_VERSION
+#include <sysdeps/unix/sysv/linux/oldglob.c>

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-13 12:22                             ` Adhemerval Zanella
@ 2017-09-14 10:05                               ` Szabolcs Nagy
  2017-09-14 13:43                                 ` Adhemerval Zanella
  0 siblings, 1 reply; 62+ messages in thread
From: Szabolcs Nagy @ 2017-09-14 10:05 UTC (permalink / raw)
  To: Adhemerval Zanella, Paul Eggert, Zack Weinberg
  Cc: nd, Joseph Myers, Andreas Schwab, GNU C Library

On 13/09/17 13:22, Adhemerval Zanella wrote:
> On 13/09/2017 06:14, Paul Eggert wrote:
>> Adhemerval Zanella wrote:
>>> Another approach that does not involve adding compat symbols (which adds
>>> a lot of code complexity inside glibc build and do not solve 'make' builds
>>> against new glibc) would to make GLOB_ALTDIRFUNCS to follow the old semantic
>>> of using gl_stat instead of gl_lstat while making glob without GLOB_ALTDIRFUNCS
>>> works as intended.  And add another flag, GLOB_ALTDIRFUNCS2, which actually
>>> uses gl_lstat.
>>
>> Although that's clever, it is a gratuitous source-code incompatibility with BSD, which is not a good thing. To some extent it's just GLOB_FOLLOW and GLOB_NOFOLLOW in disguise, and disguise is not a good thing in APIs. So I think I still prefer the compat symbol approach.
>>
>> We'll get GNU 'Make' fixed, and I wouldn't worry overly much about people building unpatched 'Make' with new glibc. I filed a Make bug report is here:
>>
>> http://lists.gnu.org/archive/html/bug-make/2017-09/msg00014.html
> 
> Right, I am mainly trying to avoid bring more internal glob implementation
> complexity to glibc, but since you says the unpatched 'Make' built
> against newer glibc shouldn't be a problem I think we can this way.
> I will work on it.
> 

i think breaking make is a serious issue now for
anyone trying to do toolchain dev (in native chroots).
and if this gets into a released version of glibc
then it will be an issue for distros.

i think old make binaries should keep working with
glibc 2.27 whatever it takes and it's best to fix
this breakage sooner than later (it's a pain to
carry patched make around).

(this is not different than the stupid malloc hook
usage was in emacs and that was dragged out over
several years until glibc was fixed to wait for
a stable emacs release that is fixed, the same
should be done for make)

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-14 10:05                               ` Szabolcs Nagy
@ 2017-09-14 13:43                                 ` Adhemerval Zanella
  0 siblings, 0 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-14 13:43 UTC (permalink / raw)
  To: Szabolcs Nagy, Paul Eggert, Zack Weinberg
  Cc: nd, Joseph Myers, Andreas Schwab, GNU C Library



On 14/09/2017 07:05, Szabolcs Nagy wrote:
> On 13/09/17 13:22, Adhemerval Zanella wrote:
>> On 13/09/2017 06:14, Paul Eggert wrote:
>>> Adhemerval Zanella wrote:
>>>> Another approach that does not involve adding compat symbols (which adds
>>>> a lot of code complexity inside glibc build and do not solve 'make' builds
>>>> against new glibc) would to make GLOB_ALTDIRFUNCS to follow the old semantic
>>>> of using gl_stat instead of gl_lstat while making glob without GLOB_ALTDIRFUNCS
>>>> works as intended.  And add another flag, GLOB_ALTDIRFUNCS2, which actually
>>>> uses gl_lstat.
>>>
>>> Although that's clever, it is a gratuitous source-code incompatibility with BSD, which is not a good thing. To some extent it's just GLOB_FOLLOW and GLOB_NOFOLLOW in disguise, and disguise is not a good thing in APIs. So I think I still prefer the compat symbol approach.
>>>
>>> We'll get GNU 'Make' fixed, and I wouldn't worry overly much about people building unpatched 'Make' with new glibc. I filed a Make bug report is here:
>>>
>>> http://lists.gnu.org/archive/html/bug-make/2017-09/msg00014.html
>>
>> Right, I am mainly trying to avoid bring more internal glob implementation
>> complexity to glibc, but since you says the unpatched 'Make' built
>> against newer glibc shouldn't be a problem I think we can this way.
>> I will work on it.
>>
> 
> i think breaking make is a serious issue now for
> anyone trying to do toolchain dev (in native chroots).
> and if this gets into a released version of glibc
> then it will be an issue for distros.
> 
> i think old make binaries should keep working with
> glibc 2.27 whatever it takes and it's best to fix
> this breakage sooner than later (it's a pain to
> carry patched make around).

Yes, that was the consensus and the idea is to provide a compat symbol
that does not call gl_lstat.

As a side note, make tests itself does not trigger it this issue
(running make tests with a newer glibc shows no regression), so it
would be good also if make adds a newer tests to stress it.

> 
> (this is not different than the stupid malloc hook
> usage was in emacs and that was dragged out over
> several years until glibc was fixed to wait for
> a stable emacs release that is fixed, the same
> should be done for make)
> 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-13  9:14                           ` Paul Eggert
  2017-09-13 12:22                             ` Adhemerval Zanella
@ 2017-09-15 20:18                             ` Florian Weimer
  2017-09-15 20:27                               ` Adhemerval Zanella
  2017-09-17  7:16                               ` Paul Eggert
  1 sibling, 2 replies; 62+ messages in thread
From: Florian Weimer @ 2017-09-15 20:18 UTC (permalink / raw)
  To: Paul Eggert, Adhemerval Zanella, Zack Weinberg
  Cc: Joseph Myers, Andreas Schwab, GNU C Library

On 09/13/2017 11:14 AM, Paul Eggert wrote:
> Although that's clever, it is a gratuitous source-code incompatibility 
> with BSD, which is not a good thing. To some extent it's just 
> GLOB_FOLLOW and GLOB_NOFOLLOW in disguise, and disguise is not a good 
> thing in APIs. So I think I still prefer the compat symbol approach.

If the BSDs are currently source-code-compatible, why doesn't GNU make 
fail there already?

Thanks,
Florian

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-15 20:18                             ` Florian Weimer
@ 2017-09-15 20:27                               ` Adhemerval Zanella
  2017-09-17  7:16                               ` Paul Eggert
  1 sibling, 0 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-15 20:27 UTC (permalink / raw)
  To: Florian Weimer, Paul Eggert, Zack Weinberg
  Cc: Joseph Myers, Andreas Schwab, GNU C Library



On 15/09/2017 17:18, Florian Weimer wrote:
> On 09/13/2017 11:14 AM, Paul Eggert wrote:
>> Although that's clever, it is a gratuitous source-code incompatibility with BSD, which is not a good thing. To some extent it's just GLOB_FOLLOW and GLOB_NOFOLLOW in disguise, and disguise is not a good thing in APIs. So I think I still prefer the compat symbol approach.
> 
> If the BSDs are currently source-code-compatible, why doesn't GNU make fail there already?

My understanding is BSDs were not current source-code-compatible before
the dangling symlink fix (commit 5554304f0) since afaik both openbsd 
and freebsd do check for dangling symlinks (using gl_lstat if it is
the case).

Reverting back to ol GLOB_ALTDIRFUNC semantic with make glibc again
source-code-incompatible and by adding an extra flag would require
adjustments in the program source code to actually handle it.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-15 20:18                             ` Florian Weimer
  2017-09-15 20:27                               ` Adhemerval Zanella
@ 2017-09-17  7:16                               ` Paul Eggert
  2017-09-17  7:48                                 ` Florian Weimer
  1 sibling, 1 reply; 62+ messages in thread
From: Paul Eggert @ 2017-09-17  7:16 UTC (permalink / raw)
  To: Florian Weimer, Adhemerval Zanella, Zack Weinberg
  Cc: Joseph Myers, Andreas Schwab, GNU C Library

Florian Weimer wrote:
> If the BSDs are currently source-code-compatible, why doesn't GNU make fail 
> there already?

Because GNU make never uses BSD glob. GNU make's 'configure' script checks that 
_GNU_GLOB_INTERFACE_VERSION equals 1, and if not it compiles and uses its own 
glob implementation (copied from an old version of glibc).

If glibc changed _GNU_GLOB_INTERFACE_VERSION to 2, old versions of GNU make 
would start rejecting new versions of glibc, and so would build and run OK 
because they'd use their old copy of glob. The comment in gnu-versions.h says 
that if we change _GNU_GLOB_INTERFACE_VERSION then we must change the libc.so 
major version, but this rule seems arbitrary.

Suppose we ignore the gnu-versions.h comment and update 
_GNU_GLOB_INTERFACE_VERSION to 2 without changing libc.so's major version. 
Wouldn't this fix the compatibility problem with GNU Make?

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-17  7:16                               ` Paul Eggert
@ 2017-09-17  7:48                                 ` Florian Weimer
  2017-09-17 14:18                                   ` Adhemerval Zanella
  0 siblings, 1 reply; 62+ messages in thread
From: Florian Weimer @ 2017-09-17  7:48 UTC (permalink / raw)
  To: Paul Eggert, Adhemerval Zanella, Zack Weinberg
  Cc: Joseph Myers, Andreas Schwab, GNU C Library

On 09/17/2017 09:16 AM, Paul Eggert wrote:
> Florian Weimer wrote:
>> If the BSDs are currently source-code-compatible, why doesn't GNU make 
>> fail there already?
> 
> Because GNU make never uses BSD glob. GNU make's 'configure' script 
> checks that _GNU_GLOB_INTERFACE_VERSION equals 1, and if not it compiles 
> and uses its own glob implementation (copied from an old version of glibc).

Ah, thanks.

> If glibc changed _GNU_GLOB_INTERFACE_VERSION to 2, old versions of GNU 
> make would start rejecting new versions of glibc, and so would build and 
> run OK because they'd use their old copy of glob. The comment in 
> gnu-versions.h says that if we change _GNU_GLOB_INTERFACE_VERSION then 
> we must change the libc.so major version, but this rule seems arbitrary.

This comment predates the availability of symbol versioning.  It was 
true when it was written.

> Suppose we ignore the gnu-versions.h comment and update 
> _GNU_GLOB_INTERFACE_VERSION to 2 without changing libc.so's major 
> version. Wouldn't this fix the compatibility problem with GNU Make?

In addition to adding a compat symbols?  Yes, that could work.

I don't really like this situation, but this combination seems to be a 
somewhat reasonable way to fix both the glob bug and preserve backwards 
compatibility.

Thanks,
Florian

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866]
  2017-09-17  7:48                                 ` Florian Weimer
@ 2017-09-17 14:18                                   ` Adhemerval Zanella
  0 siblings, 0 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-17 14:18 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Paul Eggert, Zack Weinberg, Joseph Myers, Andreas Schwab, GNU C Library



> Il giorno 17 set 2017, alle ore 04:48, Florian Weimer <fweimer@redhat.com> ha scritto:
> 
>> On 09/17/2017 09:16 AM, Paul Eggert wrote:
>> Florian Weimer wrote:
>>> If the BSDs are currently source-code-compatible, why doesn't GNU make fail there already?
>> Because GNU make never uses BSD glob. GNU make's 'configure' script checks that _GNU_GLOB_INTERFACE_VERSION equals 1, and if not it compiles and uses its own glob implementation (copied from an old version of glibc).
> 
> Ah, thanks.
> 
>> If glibc changed _GNU_GLOB_INTERFACE_VERSION to 2, old versions of GNU make would start rejecting new versions of glibc, and so would build and run OK because they'd use their old copy of glob. The comment in gnu-versions.h says that if we change _GNU_GLOB_INTERFACE_VERSION then we must change the libc.so major version, but this rule seems arbitrary.
> 
> This comment predates the availability of symbol versioning.  It was true when it was written.
> 
>> Suppose we ignore the gnu-versions.h comment and update _GNU_GLOB_INTERFACE_VERSION to 2 without changing libc.so's major version. Wouldn't this fix the compatibility problem with GNU Make?
> 
> In addition to adding a compat symbols?  Yes, that could work.
> 
> I don't really like this situation, but this combination seems to be a somewhat reasonable way to fix both the glob bug and preserve backwards compatibility.

Alright, I am also not very found on the required hacking to fix make, but I agree this seems to be the safest path. I will update the glob compat patch with minor fixes pointed out by Joseph and the _GNU_GLOB_INTERFACE_VERSION version bump.

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 4/9] Sync scratch_buffer with gnulib
  2017-09-05 20:25 ` [PATCH 4/9] Sync scratch_buffer with gnulib Adhemerval Zanella
@ 2017-09-18  6:09   ` Florian Weimer
  2017-09-18 11:43     ` Adhemerval Zanella
  0 siblings, 1 reply; 62+ messages in thread
From: Florian Weimer @ 2017-09-18  6:09 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha; +Cc: Paul Eggert

On 09/05/2017 10:25 PM, Adhemerval Zanella wrote:
> -  char __space[1024]
> -    __attribute__ ((aligned (__alignof__ (max_align_t))));
> +  max_align_t __space[(1023 + sizeof (max_align_t)) / sizeof (max_align_t)];

This change needs a declaration from the GCC folks (probably from 
Richard Biener) that it does not break aliasing analysis.  The old code 
uses a GCC extension (writing to a char array changes its dynamic type); 
it is not valid C.  I don't know if the GCC extension also applies if 
the storage site is declared with a non-char type.

Thanks,
Florian

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 4/9] Sync scratch_buffer with gnulib
  2017-09-18  6:09   ` Florian Weimer
@ 2017-09-18 11:43     ` Adhemerval Zanella
  2017-09-18 11:57       ` Florian Weimer
  0 siblings, 1 reply; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-18 11:43 UTC (permalink / raw)
  To: Florian Weimer, libc-alpha; +Cc: Paul Eggert



On 18/09/2017 03:09, Florian Weimer wrote:
> On 09/05/2017 10:25 PM, Adhemerval Zanella wrote:
>> -  char __space[1024]
>> -    __attribute__ ((aligned (__alignof__ (max_align_t))));
>> +  max_align_t __space[(1023 + sizeof (max_align_t)) / sizeof (max_align_t)];
> 
> This change needs a declaration from the GCC folks (probably from Richard Biener) that it does not break aliasing analysis.  The old code uses a GCC extension (writing to a char array changes its dynamic type); it is not valid C.  I don't know if the GCC extension also applies if the storage site is declared with a non-char type.
> 
> Thanks,
> Florian


What about this below instead:

---

diff --git a/include/scratch_buffer.h b/include/scratch_buffer.h
index bb04662..f77e7da 100644
--- a/include/scratch_buffer.h
+++ b/include/scratch_buffer.h
@@ -60,13 +60,18 @@
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdlib.h>
+#include <stdalign.h>
 
 /* Scratch buffer.  Must be initialized with scratch_buffer_init
    before its use.  */
 struct scratch_buffer {
   void *data;    /* Pointer to the beginning of the scratch area.  */
   size_t length; /* Allocated space at the data pointer, in bytes.  */
+#if __alignas_is_defined
+  _Alignas (max_align_t) char __space[1024];
+#else
   max_align_t __space[(1023 + sizeof (max_align_t)) / sizeof (max_align_t)];
+#endif
 };
 
 /* Initializes *BUFFER so that BUFFER->data points to BUFFER->__space

---

_Alignas/stdalign.h is supported since GCC 4.7 [1] (so it is safe to use
on glibc without configure support) and gnulib have its own version which
defines __alignas_is_defined depending on underlying compiler support.

[1] https://gcc.gnu.org/wiki/C11Status

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 4/9] Sync scratch_buffer with gnulib
  2017-09-18 11:43     ` Adhemerval Zanella
@ 2017-09-18 11:57       ` Florian Weimer
  2017-09-18 12:25         ` Adhemerval Zanella
  0 siblings, 1 reply; 62+ messages in thread
From: Florian Weimer @ 2017-09-18 11:57 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha; +Cc: Paul Eggert

On 09/18/2017 01:43 PM, Adhemerval Zanella wrote:
> +#if __alignas_is_defined
> +  _Alignas (max_align_t) char __space[1024];
> +#else
>     max_align_t __space[(1023 + sizeof (max_align_t)) / sizeof (max_align_t)];
> +#endif

This works for me on the glibc side.

By the way, do you have a rebased version of this patch?

Subject: [PATCH 05/18] posix: Rewrite to use struct scratch_buffer 
instead of extend_alloca

Thanks,
Florian

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH 4/9] Sync scratch_buffer with gnulib
  2017-09-18 11:57       ` Florian Weimer
@ 2017-09-18 12:25         ` Adhemerval Zanella
  0 siblings, 0 replies; 62+ messages in thread
From: Adhemerval Zanella @ 2017-09-18 12:25 UTC (permalink / raw)
  To: Florian Weimer, libc-alpha; +Cc: Paul Eggert



On 18/09/2017 08:57, Florian Weimer wrote:
> On 09/18/2017 01:43 PM, Adhemerval Zanella wrote:
>> +#if __alignas_is_defined
>> +  _Alignas (max_align_t) char __space[1024];
>> +#else
>>     max_align_t __space[(1023 + sizeof (max_align_t)) / sizeof (max_align_t)];
>> +#endif
> 
> This works for me on the glibc side.

Right, I will prepare a patch then.

> 
> By the way, do you have a rebased version of this patch?
> 
> Subject: [PATCH 05/18] posix: Rewrite to use struct scratch_buffer instead of extend_alloca
> 
> Thanks,
> Florian

No, but I can send along with some more glob fixes I plan to.

^ permalink raw reply	[flat|nested] 62+ messages in thread

end of thread, other threads:[~2017-09-18 12:25 UTC | newest]

Thread overview: 62+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-05 20:25 [PATCH 0/9] posix: glob fixes and refactor Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 9/9] posix: Fix glob with GLOB_NOCHECK returning modified patterns (BZ#10246) Adhemerval Zanella
2017-09-07 22:14   ` Paul Eggert
2017-09-08  9:16     ` Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 3/9] posix: Allow glob to match dangling symlinks [BZ #866] Adhemerval Zanella
2017-09-06  1:27   ` Paul Eggert
2017-09-06 12:57     ` Adhemerval Zanella
2017-09-09  9:50   ` Andreas Schwab
2017-09-09 11:56     ` Adhemerval Zanella
2017-09-09 17:02       ` Paul Eggert
2017-09-09 17:11         ` Zack Weinberg
2017-09-09 17:26           ` Andreas Schwab
2017-09-09 17:33             ` Zack Weinberg
2017-09-10  8:19         ` Adhemerval Zanella
2017-09-10 17:13           ` Paul Eggert
2017-09-11 14:34           ` Joseph Myers
2017-09-11 14:38             ` Zack Weinberg
2017-09-11 16:53               ` Paul Eggert
2017-09-11 17:25                 ` Zack Weinberg
2017-09-11 17:38                   ` Paul Eggert
2017-09-11 17:56                     ` Zack Weinberg
2017-09-11 18:03                       ` Paul Eggert
2017-09-11 20:09                         ` Adhemerval Zanella
2017-09-13  9:14                           ` Paul Eggert
2017-09-13 12:22                             ` Adhemerval Zanella
2017-09-14 10:05                               ` Szabolcs Nagy
2017-09-14 13:43                                 ` Adhemerval Zanella
2017-09-15 20:18                             ` Florian Weimer
2017-09-15 20:27                               ` Adhemerval Zanella
2017-09-17  7:16                               ` Paul Eggert
2017-09-17  7:48                                 ` Florian Weimer
2017-09-17 14:18                                   ` Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 6/9] posix: fix glob bugs with long login names Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 1/9] posix: Sync glob with gnulib [BZ #1062] Adhemerval Zanella
2017-09-06  2:01   ` Paul Eggert
2017-09-06 12:52     ` Adhemerval Zanella
2017-09-12 14:20   ` Andreas Schwab
2017-09-12 17:06     ` Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 7/9] posix: Consolidate glob implementation Adhemerval Zanella
2017-09-12  7:35   ` Andreas Schwab
2017-09-12 14:08     ` Adhemerval Zanella
2017-09-12 14:17       ` Andreas Schwab
2017-09-12 14:29     ` Joseph Myers
2017-09-12 14:39       ` Andreas Schwab
2017-09-12 14:50         ` Joseph Myers
2017-09-12 12:56   ` Andreas Schwab
2017-09-12 14:22     ` Adhemerval Zanella
2017-09-12 14:34       ` Andreas Schwab
2017-09-13 12:26         ` Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 2/9] posix: accept inode 0 is a valid inode number (BZ #19971) Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 8/9] posix: Use enum for __glob_pattern_type result Adhemerval Zanella
2017-09-06  1:30   ` Paul Eggert
2017-09-06  4:18   ` Paul Eggert
2017-09-06 13:04     ` Adhemerval Zanella
2017-09-06 16:18       ` Paul Eggert
2017-09-06 16:54         ` Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 4/9] Sync scratch_buffer with gnulib Adhemerval Zanella
2017-09-18  6:09   ` Florian Weimer
2017-09-18 11:43     ` Adhemerval Zanella
2017-09-18 11:57       ` Florian Weimer
2017-09-18 12:25         ` Adhemerval Zanella
2017-09-05 20:25 ` [PATCH 5/9] posix: Fix getpwnam_r usage (BZ #1062) Adhemerval Zanella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).