public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 06/12] posix: Remove alloca usage on glob user_name
  2018-02-05 13:27 [PATCH v2 00/12] posix: glob/fnmatch fixes and refactor Adhemerval Zanella
                   ` (2 preceding siblings ...)
  2018-02-05 13:27 ` [PATCH v2 03/12] posix: Remove alloca usage for GLOB_BRACE on glob Adhemerval Zanella
@ 2018-02-05 13:27 ` Adhemerval Zanella
  2018-02-05 13:28 ` [PATCH v2 07/12] posix: Use char_array for home_dir in glob Adhemerval Zanella
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 13:27 UTC (permalink / raw)
  To: libc-alpha

This patch uses dynarray at glob internal user name manipulation for
GLOB_TILDE.  It simplifies it and removes all the boilerplate buffer
managements required.  It also removes the glob_use_alloca, since
it is not used anymore.

Checked on x86_64-linux-gnu.

	* posix/glob.c (size_add_wrapv, glob_use_alloca): Remove.
	(__glob): Use char_array for user name.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 ChangeLog    |  3 +++
 posix/glob.c | 86 ++++++++++++++++++------------------------------------------
 2 files changed, 29 insertions(+), 60 deletions(-)

diff --git a/posix/glob.c b/posix/glob.c
index 2632e93..e914748 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -32,13 +32,13 @@
 
 #ifndef WINDOWS32
 # include <pwd.h>
+# include <alloca.h>
 #endif
 
 #include <errno.h>
 #include <dirent.h>
 #include <stdlib.h>
 #include <string.h>
-#include <alloca.h>
 
 #ifdef _LIBC
 # undef strdup
@@ -205,29 +205,6 @@ glob_lstat (glob_t *pglob, int flags, const char *fullname)
           : LSTAT64 (fullname, &ust.st64));
 }
 
-/* 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.  */
-
-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),
 			glob_t *pglob, size_t alloca_used);
@@ -713,11 +690,10 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
       else
 	{
 #ifndef WINDOWS32
-	  char *dirnamestr = char_array_at (&dirname, 0);
-	  char *end_name = strchr (dirnamestr, '/');
-	  char *user_name;
-	  int malloc_user_name = 0;
-	  char *unescape = NULL;
+	  const char *dirnamestr = char_array_str (&dirname);
+	  const char *end_name = strchr (dirnamestr, '/');
+	  struct char_array user_name;
+	  const char *unescape = NULL;
 
 	  if (!(flags & GLOB_NOESCAPE))
 	    {
@@ -731,27 +707,19 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		unescape = memchr (dirnamestr, '\\', end_name - dirnamestr);
 	    }
 	  if (end_name == NULL)
-	    user_name = dirnamestr + 1;
+	    {
+	      if (!char_array_init_str (&user_name, dirnamestr + 1))
+		goto err_nospace;
+	    }
 	  else
 	    {
-	      char *newp;
-	      if (glob_use_alloca (alloca_used, end_name - dirnamestr))
-		newp = alloca_account (end_name - dirnamestr, alloca_used);
-	      else
-		{
-		  newp = malloc (end_name - dirnamestr);
-		  if (newp == NULL)
-		    {
-		      retval = GLOB_NOSPACE;
-		      goto out;
-		    }
-		  malloc_user_name = 1;
-		}
 	      if (unescape != NULL)
 		{
-		  char *p = mempcpy (newp, dirnamestr + 1,
-				     unescape - dirnamestr - 1);
-		  char *q = unescape;
+		  ptrdiff_t name_len = unescape - dirnamestr - 1;
+		  if (!char_array_init_str_size (&user_name, dirnamestr + 1,
+						 name_len))
+		    goto err_nospace;
+		  const char *q = unescape;
 		  while (q != end_name)
 		    {
 		      if (*q == '\\')
@@ -762,20 +730,21 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 				 but "~fo\\o\\/" unescape to user_name
 				 "foo".  */
 			      if (filename == NULL)
-				*p++ = '\\';
+				char_array_append_char (&user_name, '\\');
 			      break;
 			    }
 			  ++q;
 			}
-		      *p++ = *q++;
+		      char_array_append_char (&user_name, *q++);
 		    }
-		  *p = '\0';
 		}
 	      else
-		*((char *) mempcpy (newp, dirnamestr + 1,
-				    end_name - dirnamestr - 1))
-		  = '\0';
-	      user_name = newp;
+		{
+		  ptrdiff_t name_len = end_name - dirnamestr - 1;
+		  if (!char_array_init_str_size (&user_name, dirnamestr + 1,
+						 name_len))
+		    goto err_nospace;
+		}
 	    }
 
 	  /* Look up specific user's home directory.  */
@@ -787,7 +756,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 #  if defined HAVE_GETPWNAM_R || defined _LIBC
 	    struct passwd pwbuf;
 
-	    while (getpwnam_r (user_name, &pwbuf,
+	    while (getpwnam_r (char_array_str (&user_name), &pwbuf,
 			       pwtmpbuf.data, pwtmpbuf.length, &p)
 		   == ERANGE)
 	      {
@@ -798,11 +767,10 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		  }
 	      }
 #  else
-	    p = getpwnam (user_name);
+	    p = getpwnam (char_array_str (&user_name));
 #  endif
 
-	    if (__glibc_unlikely (malloc_user_name))
-	      free (user_name);
+	    char_array_free (&user_name);
 
 	    /* If we found a home directory use this.  */
 	    if (p != NULL)
@@ -1025,9 +993,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
       if (meta & GLOBPAT_BACKSLASH)
 	{
 	  char *p = strchr (char_array_str (&dirname), '\\'), *q;
-	  /* We need to unescape the dirname string.  It is certainly
-	     allocated by alloca, as otherwise filename would be NULL
-	     or dirname wouldn't contain backslashes.  */
+	  /* We need to unescape the dirname string.  */
 	  q = p;
 	  do
 	    {
-- 
2.7.4

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

* [PATCH v2 00/12] posix: glob/fnmatch fixes and refactor
@ 2018-02-05 13:27 Adhemerval Zanella
  2018-02-05 13:27 ` [PATCH v2 09/12] getlogin_r: switch Linux variant to struct scratch_buffer Adhemerval Zanella
                   ` (11 more replies)
  0 siblings, 12 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 13:27 UTC (permalink / raw)
  To: libc-alpha

This patchset main target is to remove alloca usage on glob and
fnmatch by using a specialized dynarray for C strings (struct
char_array) along with already in place scratch_buffer and
dynamic_array. 

It does not change any glob semantics, only internal glob buffer
management.  It also does not solve the potential stack overflow
due recursive allocation from wildcard in patterns [1] (although
an option would be go to pure dynamic char_array for this case).

I have added the fnmatch and getlogin_r on this series because
glob uses them internally.

[1] http://lists.gnu.org/archive/html/bug-gnulib/2017-10/msg00056.html

Adhemerval Zanella (11):
  malloc: Add specialized dynarray for C strings
  posix: Use char_array for internal glob dirname
  posix: Remove alloca usage for GLOB_BRACE on glob
  posix: Remove alloca usage on glob dirname
  posix: Use dynarray for globname in glob
  posix: Remove alloca usage on glob user_name
  posix: Use char_array for home_dir in glob
  posix: Remove all alloca usage in glob
  posix: Replace alloca usage with scratch_buffer for fnmatch
  posix: Remove alloca usage for internal fnmatch implementation
  posix: Remove VLA usage for internal fnmatch implementation

Florian Weimer (1):
  getlogin_r: switch Linux variant to struct scratch_buffer

 ChangeLog                            |  47 +++
 malloc/Makefile                      |   4 +-
 malloc/Versions                      |   7 +
 malloc/char_array-impl.c             |  57 ++++
 malloc/char_array-skeleton.c         | 288 ++++++++++++++++
 malloc/char_array.h                  |  53 +++
 malloc/dynarray.h                    |   9 +
 malloc/dynarray_overflow_failure.c   |  31 ++
 malloc/malloc-internal.h             |  14 +
 malloc/tst-char_array.c              | 112 +++++++
 posix/fnmatch.c                      | 143 ++------
 posix/fnmatch_loop.c                 | 230 +++++++------
 posix/glob.c                         | 624 ++++++++++++++---------------------
 sysdeps/unix/sysv/linux/getlogin_r.c |  34 +-
 14 files changed, 1015 insertions(+), 638 deletions(-)
 create mode 100644 malloc/char_array-impl.c
 create mode 100644 malloc/char_array-skeleton.c
 create mode 100644 malloc/char_array.h
 create mode 100644 malloc/dynarray_overflow_failure.c
 create mode 100644 malloc/tst-char_array.c

-- 
2.7.4

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

* [PATCH v2 08/12] posix: Remove all alloca usage in glob
  2018-02-05 13:27 [PATCH v2 00/12] posix: glob/fnmatch fixes and refactor Adhemerval Zanella
  2018-02-05 13:27 ` [PATCH v2 09/12] getlogin_r: switch Linux variant to struct scratch_buffer Adhemerval Zanella
@ 2018-02-05 13:27 ` Adhemerval Zanella
  2018-02-05 13:27 ` [PATCH v2 03/12] posix: Remove alloca usage for GLOB_BRACE on glob Adhemerval Zanella
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 13:27 UTC (permalink / raw)
  To: libc-alpha

With alloca usage removal from glob this patch wraps it up by removing
all the alloca defines and macros usage.

Checked on x86_64-linux-gnu.

	* posix/glob.c (glob_in_dir, glob): Remove alloca_used argument.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 ChangeLog    |  2 ++
 posix/glob.c | 25 ++++++++-----------------
 2 files changed, 10 insertions(+), 17 deletions(-)

diff --git a/posix/glob.c b/posix/glob.c
index a78718d..118fcf5 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -32,7 +32,6 @@
 
 #ifndef WINDOWS32
 # include <pwd.h>
-# include <alloca.h>
 #endif
 
 #include <errno.h>
@@ -65,9 +64,6 @@
 # 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 */
@@ -174,12 +170,6 @@ convert_dirent64 (const struct dirent64 *source)
 # 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
 
 static int
@@ -207,7 +197,7 @@ glob_lstat (glob_t *pglob, int flags, const char *fullname)
 
 static int glob_in_dir (const char *pattern, const char *directory,
 			int flags, int (*errfunc) (const char *, int),
-			glob_t *pglob, size_t alloca_used);
+			glob_t *pglob);
 static int prefix_array (const char *prefix, char **array, size_t n) __THROWNL;
 static int collated_compare (const void *, const void *) __THROWNL;
 
@@ -273,7 +263,6 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
   bool dirname_modified;
   glob_t dirs;
   int retval = 0;
-  size_t alloca_used = 0;
   struct char_array dirname;
 
   if (pattern == NULL || pglob == NULL || (flags & ~__GLOB_FLAGS) != 0)
@@ -520,11 +509,13 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  char *drive_spec;
 
 	  ++dirlen;
-	  drive_spec = __alloca (dirlen + 1);
+	  drive_spec = malloc (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)))
+	  int r = __glob_pattern_p (drive_spec, !(flags & GLOB_NOESCAPE));
+	  free (drive_spec);
+	  if (r != 0)
 	    {
 	      retval = GLOB_NOMATCH;
 	      goto out;
@@ -921,7 +912,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  status = glob_in_dir (filename, dirs.gl_pathv[i],
 				((flags | GLOB_APPEND)
 				 & ~(GLOB_NOCHECK | GLOB_NOMAGIC)),
-				errfunc, pglob, alloca_used);
+				errfunc, pglob);
 	  if (status == GLOB_NOMATCH)
 	    /* No matches in this directory.  Try the next.  */
 	    continue;
@@ -1026,7 +1017,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
       if (dirname_modified)
 	flags &= ~(GLOB_NOCHECK | GLOB_NOMAGIC);
       status = glob_in_dir (filename, char_array_str (&dirname), flags,
-			    errfunc, pglob, alloca_used);
+			    errfunc, pglob);
       if (status != 0)
 	{
 	  if (status == GLOB_NOMATCH && flags != orig_flags
@@ -1191,7 +1182,7 @@ struct globnames_result
 static int
 glob_in_dir (const char *pattern, const char *directory, int flags,
 	     int (*errfunc) (const char *, int),
-	     glob_t *pglob, size_t alloca_used)
+	     glob_t *pglob)
 {
   void *stream = NULL;
   struct globnames_array globnames;
-- 
2.7.4

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

* [PATCH v2 09/12] getlogin_r: switch Linux variant to struct scratch_buffer
  2018-02-05 13:27 [PATCH v2 00/12] posix: glob/fnmatch fixes and refactor Adhemerval Zanella
@ 2018-02-05 13:27 ` Adhemerval Zanella
  2018-02-05 14:10   ` Florian Weimer
  2018-02-05 13:27 ` [PATCH v2 08/12] posix: Remove all alloca usage in glob Adhemerval Zanella
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 13:27 UTC (permalink / raw)
  To: libc-alpha; +Cc: Florian Weimer

From: Florian Weimer <fweimer@redhat.com>

	[BZ #18023]
	* sysdeps/unix/sysv/linux/getlogin_r.c (__getlogin_r_loginuid):
	Use scratch_buffer instead of extend_alloca.
---
 ChangeLog                            |  6 ++++++
 sysdeps/unix/sysv/linux/getlogin_r.c | 34 +++++++++++++---------------------
 2 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/sysdeps/unix/sysv/linux/getlogin_r.c b/sysdeps/unix/sysv/linux/getlogin_r.c
index 84c51d0..73ea14c 100644
--- a/sysdeps/unix/sysv/linux/getlogin_r.c
+++ b/sysdeps/unix/sysv/linux/getlogin_r.c
@@ -18,6 +18,7 @@
 #include <pwd.h>
 #include <unistd.h>
 #include <not-cancel.h>
+#include <scratch_buffer.h>
 
 #define STATIC static
 static int getlogin_r_fd0 (char *name, size_t namesize);
@@ -54,29 +55,22 @@ __getlogin_r_loginuid (char *name, size_t namesize)
 	  endp == uidbuf || *endp != '\0'))
     return -1;
 
-  size_t buflen = 1024;
-  char *buf = alloca (buflen);
-  bool use_malloc = false;
   struct passwd pwd;
   struct passwd *tpwd;
   int result = 0;
   int res;
+  struct scratch_buffer tmpbuf;
+  scratch_buffer_init (&tmpbuf);
 
-  while ((res = __getpwuid_r (uid, &pwd, buf, buflen, &tpwd)) == ERANGE)
-    if (__libc_use_alloca (2 * buflen))
-      buf = extend_alloca (buf, buflen, 2 * buflen);
-    else
-      {
-	buflen *= 2;
-	char *newp = realloc (use_malloc ? buf : NULL, buflen);
-	if (newp == NULL)
-	  {
-	    result = ENOMEM;
-	    goto out;
-	  }
-	buf = newp;
-	use_malloc = true;
-      }
+  while ((res =  __getpwuid_r (uid, &pwd,
+			       tmpbuf.data, tmpbuf.length, &tpwd)) == ERANGE)
+    {
+      if (!scratch_buffer_grow (&tmpbuf))
+	{
+	  result = ENOMEM;
+	  goto out;
+	}
+    }
 
   if (res != 0 || tpwd == NULL)
     {
@@ -95,9 +89,7 @@ __getlogin_r_loginuid (char *name, size_t namesize)
   memcpy (name, pwd.pw_name, needed);
 
  out:
-  if (use_malloc)
-    free (buf);
-
+  scratch_buffer_free (&tmpbuf);
   return result;
 }
 
-- 
2.7.4

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

* [PATCH v2 03/12] posix: Remove alloca usage for GLOB_BRACE on glob
  2018-02-05 13:27 [PATCH v2 00/12] posix: glob/fnmatch fixes and refactor Adhemerval Zanella
  2018-02-05 13:27 ` [PATCH v2 09/12] getlogin_r: switch Linux variant to struct scratch_buffer Adhemerval Zanella
  2018-02-05 13:27 ` [PATCH v2 08/12] posix: Remove all alloca usage in glob Adhemerval Zanella
@ 2018-02-05 13:27 ` Adhemerval Zanella
  2018-02-05 13:27 ` [PATCH v2 06/12] posix: Remove alloca usage on glob user_name Adhemerval Zanella
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 13:27 UTC (permalink / raw)
  To: libc-alpha

GNU GLOB_BRACE internal implementation constructs a new expression and
calls glob recursively.  It then requires a possible large temporary
buffer place the new pattern.

This patch removes the alloca/malloc usage and replaces it with
char_array.

Checked on x86_64-linux-gnu.

	* posix/glob.c (glob): Remove alloca usage for onealt.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 ChangeLog    |  2 ++
 posix/glob.c | 53 +++++++++++++++++++++++++++++------------------------
 2 files changed, 31 insertions(+), 24 deletions(-)

diff --git a/posix/glob.c b/posix/glob.c
index 4814d67..3e36eeb 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -372,25 +372,23 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  /* Allocate working buffer large enough for our work.  Note that
 	    we have at least an opening and closing brace.  */
 	  size_t firstc;
-	  char *alt_start;
 	  const char *p;
 	  const char *next;
 	  const char *rest;
 	  size_t rest_len;
-	  char *onealt;
-	  size_t pattern_len = strlen (pattern) - 1;
-	  int alloca_onealt = glob_use_alloca (alloca_used, pattern_len);
-	  if (alloca_onealt)
-	    onealt = alloca_account (pattern_len, alloca_used);
-	  else
-	    {
-	      onealt = malloc (pattern_len);
-	      if (onealt == NULL)
-		goto err_nospace;
-	    }
+	  struct char_array onealt;
 
 	  /* We know the prefix for all sub-patterns.  */
-	  alt_start = mempcpy (onealt, pattern, begin - pattern);
+	  ptrdiff_t onealtlen = begin - pattern;
+	  if (!char_array_init_str_size (&onealt, pattern, onealtlen))
+	    {
+	      if (!(flags & GLOB_APPEND))
+		{
+		  pglob->gl_pathc = 0;
+		  pglob->gl_pathv = NULL;
+		}
+	      goto err_nospace;
+	    }
 
 	  /* Find the first sub-pattern and at the same time find the
 	     rest after the closing brace.  */
@@ -398,9 +396,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  if (next == NULL)
 	    {
 	      /* It is an invalid expression.  */
-	    illegal_brace:
-	      if (__glibc_unlikely (!alloca_onealt))
-		free (onealt);
+	      char_array_free (&onealt);
 	      flags &= ~GLOB_BRACE;
 	      goto no_brace;
 	    }
@@ -411,8 +407,12 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	    {
 	      rest = next_brace_sub (rest + 1, flags);
 	      if (rest == NULL)
-		/* It is an illegal expression.  */
-		goto illegal_brace;
+		{
+		  /* It is an illegal expression.  */
+		  char_array_free (&onealt);
+		  flags &= ~GLOB_BRACE;
+		  goto no_brace;
+		}
 	    }
 	  /* Please note that we now can be sure the brace expression
 	     is well-formed.  */
@@ -431,9 +431,16 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      int result;
 
 	      /* Construct the new glob expression.  */
-	      mempcpy (mempcpy (alt_start, p, next - p), rest, rest_len);
+	      ptrdiff_t nextlen = next - p;
+	      if (!char_array_replace_str_pos (&onealt, onealtlen, p, nextlen)
+		  || !char_array_replace_str_pos (&onealt, onealtlen + nextlen,
+						  rest, rest_len))
+		{
+		  char_array_free (&onealt);
+		  goto err_nospace;
+		}
 
-	      result = __glob (onealt,
+	      result = __glob (char_array_str (&onealt),
 			       ((flags & ~(GLOB_NOCHECK | GLOB_NOMAGIC))
 				| GLOB_APPEND),
 			       errfunc, pglob);
@@ -441,8 +448,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      /* If we got an error, return it.  */
 	      if (result && result != GLOB_NOMATCH)
 		{
-		  if (__glibc_unlikely (!alloca_onealt))
-		    free (onealt);
+		  char_array_free (&onealt);
 		  if (!(flags & GLOB_APPEND))
 		    {
 		      globfree (pglob);
@@ -461,8 +467,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      assert (next != NULL);
 	    }
 
-	  if (__glibc_unlikely (!alloca_onealt))
-	    free (onealt);
+	  char_array_free (&onealt);
 
 	  if (pglob->gl_pathc != firstc)
 	    /* We found some entries.  */
-- 
2.7.4

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

* [PATCH v2 10/12] posix: Replace alloca usage with scratch_buffer for fnmatch
  2018-02-05 13:27 [PATCH v2 00/12] posix: glob/fnmatch fixes and refactor Adhemerval Zanella
                   ` (9 preceding siblings ...)
  2018-02-05 13:28 ` [PATCH v2 12/12] posix: Remove VLA usage for internal fnmatch implementation Adhemerval Zanella
@ 2018-02-05 13:28 ` Adhemerval Zanella
  2018-02-05 13:40 ` [PATCH v2 02/12] posix: Use char_array for internal glob dirname Adhemerval Zanella
  11 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 13:28 UTC (permalink / raw)
  To: libc-alpha

Checked on x86_64-linux-gnu.

	* posix/fnmatch.c (fnmatch): Use scratch_buffer instead of
	alloca.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 ChangeLog       |   5 ++
 posix/fnmatch.c | 140 ++++++++++----------------------------------------------
 2 files changed, 29 insertions(+), 116 deletions(-)

diff --git a/posix/fnmatch.c b/posix/fnmatch.c
index a9b7626..4be7327 100644
--- a/posix/fnmatch.c
+++ b/posix/fnmatch.c
@@ -34,11 +34,7 @@
 # include <stdlib.h>
 #endif
 
-#ifdef _LIBC
-# include <alloca.h>
-#else
-# define alloca_account(size., var) alloca (size)
-#endif
+#include <scratch_buffer.h>
 
 /* For platform which support the ISO C amendement 1 functionality we
    support user defined character classes.  */
@@ -315,128 +311,40 @@ is_char_class (const wchar_t *wcs)
 #  include "fnmatch_loop.c"
 # endif
 
-
 int
 fnmatch (const char *pattern, const char *string, int flags)
 {
 # if HANDLE_MULTIBYTE
-  if (__builtin_expect (MB_CUR_MAX, 1) != 1)
+  if (__glibc_unlikely (MB_CUR_MAX != 1))
     {
-      mbstate_t ps;
-      size_t n;
-      const char *p;
-      wchar_t *wpattern_malloc = NULL;
-      wchar_t *wpattern;
-      wchar_t *wstring_malloc = NULL;
-      wchar_t *wstring;
-      size_t alloca_used = 0;
+      mbstate_t ps = { 0 };
 
-      /* Convert the strings into wide characters.  */
-      memset (&ps, '\0', sizeof (ps));
-      p = pattern;
-#ifdef _LIBC
-      n = __strnlen (pattern, 1024);
-#else
-      n = strlen (pattern);
-#endif
-      if (__glibc_likely (n < 1024))
-	{
-	  wpattern = (wchar_t *) alloca_account ((n + 1) * sizeof (wchar_t),
-						 alloca_used);
-	  n = mbsrtowcs (wpattern, &p, n + 1, &ps);
-	  if (__glibc_unlikely (n == (size_t) -1))
-	    /* Something wrong.
-	       XXX Do we have to set `errno' to something which mbsrtows hasn't
-	       already done?  */
-	    return -1;
-	  if (p)
-	    {
-	      memset (&ps, '\0', sizeof (ps));
-	      goto prepare_wpattern;
-	    }
-	}
-      else
-	{
-	prepare_wpattern:
-	  n = mbsrtowcs (NULL, &pattern, 0, &ps);
-	  if (__glibc_unlikely (n == (size_t) -1))
-	    /* Something wrong.
-	       XXX Do we have to set `errno' to something which mbsrtows hasn't
-	       already done?  */
-	    return -1;
-	  if (__glibc_unlikely (n >= (size_t) -1 / sizeof (wchar_t)))
-	    {
-	      __set_errno (ENOMEM);
-	      return -2;
-	    }
-	  wpattern_malloc = wpattern
-	    = (wchar_t *) malloc ((n + 1) * sizeof (wchar_t));
-	  assert (mbsinit (&ps));
-	  if (wpattern == NULL)
-	    return -2;
-	  (void) mbsrtowcs (wpattern, &pattern, n + 1, &ps);
-	}
+      /* Calculate the size needed to convert the strings to wide
+	 characters.  */
+      size_t patsize = mbsrtowcs (NULL, &pattern, 0, &ps) + 1;
+      size_t strsize = mbsrtowcs (NULL, &string, 0, &ps) + 1;
+      size_t totsize = patsize + strsize;
 
-      assert (mbsinit (&ps));
-#ifdef _LIBC
-      n = __strnlen (string, 1024);
-#else
-      n = strlen (string);
-#endif
-      p = string;
-      if (__glibc_likely (n < 1024))
-	{
-	  wstring = (wchar_t *) alloca_account ((n + 1) * sizeof (wchar_t),
-						alloca_used);
-	  n = mbsrtowcs (wstring, &p, n + 1, &ps);
-	  if (__glibc_unlikely (n == (size_t) -1))
-	    {
-	      /* Something wrong.
-		 XXX Do we have to set `errno' to something which
-		 mbsrtows hasn't already done?  */
-	    free_return:
-	      free (wpattern_malloc);
-	      return -1;
-	    }
-	  if (p)
-	    {
-	      memset (&ps, '\0', sizeof (ps));
-	      goto prepare_wstring;
-	    }
-	}
-      else
+      struct scratch_buffer s;
+      scratch_buffer_init (&s);
+      if (!scratch_buffer_set_array_size (&s, totsize, sizeof (wchar_t)))
 	{
-	prepare_wstring:
-	  n = mbsrtowcs (NULL, &string, 0, &ps);
-	  if (__glibc_unlikely (n == (size_t) -1))
-	    /* Something wrong.
-	       XXX Do we have to set `errno' to something which mbsrtows hasn't
-	       already done?  */
-	    goto free_return;
-	  if (__glibc_unlikely (n >= (size_t) -1 / sizeof (wchar_t)))
-	    {
-	      free (wpattern_malloc);
-	      __set_errno (ENOMEM);
-	      return -2;
-	    }
-
-	  wstring_malloc = wstring
-	    = (wchar_t *) malloc ((n + 1) * sizeof (wchar_t));
-	  if (wstring == NULL)
-	    {
-	      free (wpattern_malloc);
-	      return -2;
-	    }
-	  assert (mbsinit (&ps));
-	  (void) mbsrtowcs (wstring, &string, n + 1, &ps);
+	  scratch_buffer_free (&s);
+	  errno = ENOMEM;
+	  return -1;
 	}
 
-      int res = internal_fnwmatch (wpattern, wstring, wstring + n,
-				   flags & FNM_PERIOD, flags, NULL,
-				   alloca_used);
+      wchar_t *wpattern = s.data;
+      wchar_t *wstring = wpattern + patsize;
+
+      /* Convert the strings into wide characters.  */
+      mbsrtowcs (wpattern, &pattern, patsize, &ps);
+      mbsrtowcs (wstring, &string, strsize, &ps);
+
+      int res = internal_fnwmatch (wpattern, wstring, wstring + strsize - 1,
+				   flags & FNM_PERIOD, flags, NULL, 0);
 
-      free (wstring_malloc);
-      free (wpattern_malloc);
+      scratch_buffer_free (&s);
 
       return res;
     }
-- 
2.7.4

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

* [PATCH v2 05/12] posix: Use dynarray for globname in glob
  2018-02-05 13:27 [PATCH v2 00/12] posix: glob/fnmatch fixes and refactor Adhemerval Zanella
                   ` (5 preceding siblings ...)
  2018-02-05 13:28 ` [PATCH v2 01/12] malloc: Add specialized dynarray for C strings Adhemerval Zanella
@ 2018-02-05 13:28 ` Adhemerval Zanella
  2018-02-05 13:28 ` [PATCH v2 11/12] posix: Remove alloca usage for internal fnmatch implementation Adhemerval Zanella
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 13:28 UTC (permalink / raw)
  To: libc-alpha

This patch uses dynarray at glob internal glob_in_dir function to manage
the various matched patterns.  It simplify and removes all the boilerplate
buffer managements required.  It also removes the glob_use_alloca, since
it is not used anymore.

Checked on x86_64-linux-gnu.

	* posix/glob.c (glob_in_dir): Use dynarray for globnames.
---
 ChangeLog    |   2 +
 posix/glob.c | 127 +++++++++++++++++------------------------------------------
 2 files changed, 39 insertions(+), 90 deletions(-)

diff --git a/posix/glob.c b/posix/glob.c
index 26af331..2632e93 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -1189,6 +1189,21 @@ prefix_array (const char *dirname, char **array, size_t n)
   return 0;
 }
 
+struct globnames_result
+{
+  char **names;
+  size_t length;
+};
+
+/* Create a dynamic array for C string representing the glob name found.  */
+#define DYNARRAY_STRUCT            globnames_array
+#define DYNARRAY_ELEMENT_FREE(ptr) free (*ptr)
+#define DYNARRAY_ELEMENT           char *
+#define DYNARRAY_PREFIX            globnames_array_
+#define DYNARRAY_FINAL_TYPE        struct globnames_result
+#define DYNARRAY_INITIAL_SIZE      64
+#include <malloc/dynarray-skeleton.c>
+
 /* 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.
@@ -1199,25 +1214,13 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 	     glob_t *pglob, size_t alloca_used)
 {
   void *stream = NULL;
-# 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;
+  struct globnames_array globnames;
   size_t nfound = 0;
-  size_t cur = 0;
   int meta;
   int save;
   int result;
 
-  alloca_used += sizeof init_names_buf;
-
-  init_names->next = NULL;
-  init_names->count = ((sizeof init_names_buf
-                        - offsetof (struct globnames, name))
-                       / sizeof init_names->name[0]);
+  globnames_array_init (&globnames);
 
   meta = __glob_pattern_type (pattern, !(flags & GLOB_NOESCAPE));
   if (meta == GLOBPAT_NONE && (flags & (GLOB_NOCHECK|GLOB_NOMAGIC)))
@@ -1294,34 +1297,10 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 
 	      if (fnmatch (pattern, d.name, fnm_flags) == 0)
 		{
-		  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)
-		    goto memory_error;
-		  ++cur;
-		  ++nfound;
-		  if (SIZE_MAX - pglob->gl_offs <= nfound)
+		  globnames_array_add (&globnames, strdup (d.name));
+		  if (globnames_array_has_failed (&globnames))
 		    goto memory_error;
+		  nfound++;
 		}
 	    }
 	}
@@ -1331,10 +1310,13 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
     {
       size_t len = strlen (pattern);
       nfound = 1;
-      names->name[cur] = malloc (len + 1);
-      if (names->name[cur] == NULL)
+      char *newp = malloc (len + 1);
+      if (newp == NULL)
+	goto memory_error;
+      *((char *) mempcpy (newp, pattern, len)) = '\0';
+      globnames_array_add (&globnames, newp);
+      if (globnames_array_has_failed (&globnames))
 	goto memory_error;
-      *((char *) mempcpy (names->name[cur++], pattern, len)) = '\0';
     }
 
   result = GLOB_NOMATCH;
@@ -1355,59 +1337,24 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
       if (new_gl_pathv == NULL)
 	{
 	memory_error:
-	  while (1)
-	    {
-	      struct globnames *old = names;
-	      for (size_t i = 0; i < cur; ++i)
-		free (names->name[i]);
-	      names = names->next;
-	      /* NB: we will not leak memory here if we exit without
-		 freeing the current block assigned to OLD.  At least
-		 the very first block is always allocated on the stack
-		 and this is the block assigned to OLD here.  */
-	      if (names == NULL)
-		{
-		  assert (old == init_names);
-		  break;
-		}
-	      cur = names->count;
-	      if (old == names_alloca)
-		names_alloca = names;
-	      else
-		free (old);
-	    }
+	  globnames_array_free (&globnames);
 	  result = GLOB_NOSPACE;
 	}
       else
 	{
-	  while (1)
+	  struct globnames_result ret = { .names = 0, .length = -1 };
+	  if (!globnames_array_finalize (&globnames, &ret))
+	    result = GLOB_NOSPACE;
+	  else
 	    {
-	      struct globnames *old = names;
-	      for (size_t i = 0; i < cur; ++i)
+	      for (size_t i = 0; i < ret.length; ++i)
 		new_gl_pathv[pglob->gl_offs + pglob->gl_pathc++]
-		  = names->name[i];
-	      names = names->next;
-	      /* NB: we will not leak memory here if we exit without
-		 freeing the current block assigned to OLD.  At least
-		 the very first block is always allocated on the stack
-		 and this is the block assigned to OLD here.  */
-	      if (names == NULL)
-		{
-		  assert (old == init_names);
-		  break;
-		}
-	      cur = names->count;
-	      if (old == names_alloca)
-		names_alloca = names;
-	      else
-		free (old);
+		  = ret.names[i];
+	      pglob->gl_pathv = new_gl_pathv;
+	      pglob->gl_pathv[pglob->gl_offs + pglob->gl_pathc] = NULL;
+	      pglob->gl_flags = flags;
 	    }
-
-	  pglob->gl_pathv = new_gl_pathv;
-
-	  pglob->gl_pathv[pglob->gl_offs + pglob->gl_pathc] = NULL;
-
-	  pglob->gl_flags = flags;
+	  free (ret.names);
 	}
     }
 
-- 
2.7.4

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

* [PATCH v2 12/12] posix: Remove VLA usage for internal fnmatch implementation
  2018-02-05 13:27 [PATCH v2 00/12] posix: glob/fnmatch fixes and refactor Adhemerval Zanella
                   ` (8 preceding siblings ...)
  2018-02-05 13:28 ` [PATCH v2 04/12] posix: Remove alloca usage on glob dirname Adhemerval Zanella
@ 2018-02-05 13:28 ` Adhemerval Zanella
  2018-02-05 14:01   ` Andreas Schwab
  2018-02-05 13:28 ` [PATCH v2 10/12] posix: Replace alloca usage with scratch_buffer for fnmatch Adhemerval Zanella
  2018-02-05 13:40 ` [PATCH v2 02/12] posix: Use char_array for internal glob dirname Adhemerval Zanella
  11 siblings, 1 reply; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 13:28 UTC (permalink / raw)
  To: libc-alpha

Checked on x86_64-linux-gnu.

	* posix/fnmatch.c: Include char_array required files.
	* posix/fnmatch_loop.c (FCT): Replace VLA with char_array.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 ChangeLog            |  3 +++
 posix/fnmatch.c      |  1 +
 posix/fnmatch_loop.c | 68 +++++++++++++++++++++++++---------------------------
 3 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/posix/fnmatch.c b/posix/fnmatch.c
index 9c2cff0..b94c965 100644
--- a/posix/fnmatch.c
+++ b/posix/fnmatch.c
@@ -35,6 +35,7 @@
 #endif
 
 #include <scratch_buffer.h>
+#include <malloc/char_array-skeleton.c>
 
 /* For platform which support the ISO C amendement 1 functionality we
    support user defined character classes.  */
diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c
index eadb343..831e5ba 100644
--- a/posix/fnmatch_loop.c
+++ b/posix/fnmatch_loop.c
@@ -493,26 +493,20 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 			  {
 			    int32_t table_size;
 			    const int32_t *symb_table;
-# if WIDE_CHAR_VERSION
-			    char str[c1];
-			    unsigned int strcnt;
-# else
-#  define str (startp + 1)
-# endif
+			    struct char_array str;
+			    char_array_init_empty (&str);
 			    const unsigned char *extra;
 			    int32_t idx;
 			    int32_t elem;
 			    int32_t second;
 			    int32_t hash;
 
-# if WIDE_CHAR_VERSION
 			    /* We have to convert the name to a single-byte
 			       string.  This is possible since the names
 			       consist of ASCII characters and the internal
 			       representation is UCS4.  */
-			    for (strcnt = 0; strcnt < c1; ++strcnt)
-			      str[strcnt] = startp[1 + strcnt];
-#endif
+			    for (size_t strcnt = 0; strcnt < c1; ++strcnt)
+			      char_array_append_char (&str, startp[1 + strcnt]);
 
 			    table_size =
 			      _NL_CURRENT_WORD (LC_COLLATE,
@@ -525,7 +519,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 					   _NL_COLLATE_SYMB_EXTRAMB);
 
 			    /* Locate the character in the hashing table.  */
-			    hash = elem_hash (str, c1);
+			    hash = elem_hash (char_array_str (&str), c1);
 
 			    idx = 0;
 			    elem = hash % table_size;
@@ -539,7 +533,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 				    if (symb_table[2 * elem] == hash
 					&& (c1
 					    == extra[symb_table[2 * elem + 1]])
-					&& memcmp (str,
+					&& memcmp (char_array_str (&str),
 						   &extra[symb_table[2 * elem
 								     + 1]
 							  + 1], c1) == 0)
@@ -580,14 +574,20 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 					break;
 
 				    if ((int32_t) c1 == wextra[idx])
-				      goto matched;
+				      {
+					char_array_free (&str);
+				        goto matched;
+				      }
 # else
 				    for (c1 = 0; c1 < extra[idx]; ++c1)
 				      if (n[c1] != extra[1 + c1])
 					break;
 
 				    if (c1 == extra[idx])
-				      goto matched;
+				      {
+					char_array_free (&str);
+				        goto matched;
+				      }
 # endif
 				  }
 
@@ -608,18 +608,21 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 			      {
 				/* No valid character.  Match it as a
 				   single byte.  */
-				if (!is_range && *n == str[0])
-				  goto matched;
+				char str0 = *char_array_at (&str, 0);
+				if (!is_range && *n == str0)
+				  {
+				    char_array_free (&str);
+				    goto matched;
+				  }
 
-				cold = str[0];
+				cold = str0;
 				c = *p++;
 			      }
-			    else
-			      return FNM_NOMATCH;
+			    char_array_free (&str);
+			    return FNM_NOMATCH;
 			  }
 		      }
 		    else
-# undef str
 #endif
 		      {
 			c = FOLD (c);
@@ -711,26 +714,20 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 			      {
 				int32_t table_size;
 				const int32_t *symb_table;
-# if WIDE_CHAR_VERSION
-				char str[c1];
-				unsigned int strcnt;
-# else
-#  define str (startp + 1)
-# endif
+				struct char_array str;
+				char_array_init_empty (&str);
 				const unsigned char *extra;
 				int32_t idx;
 				int32_t elem;
 				int32_t second;
 				int32_t hash;
 
-# if WIDE_CHAR_VERSION
 				/* We have to convert the name to a single-byte
 				   string.  This is possible since the names
 				   consist of ASCII characters and the internal
 				   representation is UCS4.  */
-				for (strcnt = 0; strcnt < c1; ++strcnt)
-				  str[strcnt] = startp[1 + strcnt];
-# endif
+				for (size_t strcnt = 0; strcnt < c1; ++strcnt)
+				  char_array_append_char (&str, startp[1 + strcnt]);
 
 				table_size =
 				  _NL_CURRENT_WORD (LC_COLLATE,
@@ -744,7 +741,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 
 				/* Locate the character in the hashing
 				   table.  */
-				hash = elem_hash (str, c1);
+				hash = elem_hash (char_array_str (&str), c1);
 
 				idx = 0;
 				elem = hash % table_size;
@@ -758,7 +755,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 					if (symb_table[2 * elem] == hash
 					    && (c1
 						== extra[symb_table[2 * elem + 1]])
-					    && memcmp (str,
+					    && memcmp (char_array_str (&str),
 						       &extra[symb_table[2 * elem + 1]
 							      + 1], c1) == 0)
 					  {
@@ -800,13 +797,12 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 				  }
 				else if (symb_table[2 * elem] != 0 && c1 == 1)
 				  {
-				    cend = str[0];
+				    cend = *char_array_at (&str, 0);
 				    c = *p++;
 				  }
-				else
-				  return FNM_NOMATCH;
+				char_array_free (&str);
+				return FNM_NOMATCH;
 			      }
-# undef str
 			  }
 			else
 			  {
-- 
2.7.4

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

* [PATCH v2 11/12] posix: Remove alloca usage for internal fnmatch implementation
  2018-02-05 13:27 [PATCH v2 00/12] posix: glob/fnmatch fixes and refactor Adhemerval Zanella
                   ` (6 preceding siblings ...)
  2018-02-05 13:28 ` [PATCH v2 05/12] posix: Use dynarray for globname in glob Adhemerval Zanella
@ 2018-02-05 13:28 ` Adhemerval Zanella
  2018-02-05 13:28 ` [PATCH v2 04/12] posix: Remove alloca usage on glob dirname Adhemerval Zanella
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 13:28 UTC (permalink / raw)
  To: libc-alpha

This patch replaces the internal fnmatch pattern list generation from
a linked list using alloca to a dynamic array.

Checked on x86_64-linux-gnu.

	* posix/fnmatch_loop.c (FCT, EXT): Remove alloca_used argument and
	use dynamic_array for pattern list.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 ChangeLog            |   3 +
 posix/fnmatch.c      |   4 +-
 posix/fnmatch_loop.c | 162 +++++++++++++++++++++++++--------------------------
 3 files changed, 86 insertions(+), 83 deletions(-)

diff --git a/posix/fnmatch.c b/posix/fnmatch.c
index 4be7327..9c2cff0 100644
--- a/posix/fnmatch.c
+++ b/posix/fnmatch.c
@@ -342,7 +342,7 @@ fnmatch (const char *pattern, const char *string, int flags)
       mbsrtowcs (wstring, &string, strsize, &ps);
 
       int res = internal_fnwmatch (wpattern, wstring, wstring + strsize - 1,
-				   flags & FNM_PERIOD, flags, NULL, 0);
+				   flags & FNM_PERIOD, flags, NULL);
 
       scratch_buffer_free (&s);
 
@@ -351,7 +351,7 @@ fnmatch (const char *pattern, const char *string, int flags)
 # endif  /* mbstate_t and mbsrtowcs or _LIBC.  */
 
   return internal_fnmatch (pattern, string, string + strlen (string),
-			   flags & FNM_PERIOD, flags, NULL, 0);
+			   flags & FNM_PERIOD, flags, NULL);
 }
 
 # ifdef _LIBC
diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c
index e298cac..eadb343 100644
--- a/posix/fnmatch_loop.c
+++ b/posix/fnmatch_loop.c
@@ -28,15 +28,14 @@ struct STRUCT
    it matches, nonzero if not.  */
 static int FCT (const CHAR *pattern, const CHAR *string,
 		const CHAR *string_end, int no_leading_period, int flags,
-		struct STRUCT *ends, size_t alloca_used);
+		struct STRUCT *ends);
 static int EXT (INT opt, const CHAR *pattern, const CHAR *string,
-		const CHAR *string_end, int no_leading_period, int flags,
-		size_t alloca_used);
+		const CHAR *string_end, int no_leading_period, int flags);
 static const CHAR *END (const CHAR *patternp);
 
 static int
 FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
-     int no_leading_period, int flags, struct STRUCT *ends, size_t alloca_used)
+     int no_leading_period, int flags, struct STRUCT *ends)
 {
   const CHAR *p = pattern, *n = string;
   UCHAR c;
@@ -61,7 +60,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 	  if (__builtin_expect (flags & FNM_EXTMATCH, 0) && *p == '(')
 	    {
 	      int res = EXT (c, p, n, string_end, no_leading_period,
-			     flags, alloca_used);
+			     flags);
 	      if (res != -1)
 		return res;
 	    }
@@ -91,7 +90,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 	  if (__builtin_expect (flags & FNM_EXTMATCH, 0) && *p == '(')
 	    {
 	      int res = EXT (c, p, n, string_end, no_leading_period,
-			     flags, alloca_used);
+			     flags);
 	      if (res != -1)
 		return res;
 	    }
@@ -180,7 +179,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 
 		  for (--p; n < endp; ++n, no_leading_period = 0)
 		    if (FCT (p, n, string_end, no_leading_period, flags2,
-			     &end, alloca_used) == 0)
+			     &end) == 0)
 		      goto found;
 		}
 	      else if (c == L('/') && (flags & FNM_FILE_NAME))
@@ -189,7 +188,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 		    ++n;
 		  if (n < string_end && *n == L('/')
 		      && (FCT (p, n + 1, string_end, flags & FNM_PERIOD, flags,
-			       NULL, alloca_used) == 0))
+			       NULL) == 0))
 		    return 0;
 		}
 	      else
@@ -203,7 +202,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 		  for (--p; n < endp; ++n, no_leading_period = 0)
 		    if (FOLD ((UCHAR) *n) == c
 			&& (FCT (p, n, string_end, no_leading_period, flags2,
-				 &end, alloca_used) == 0))
+				 &end) == 0))
 		      {
 		      found:
 			if (end.pattern == NULL)
@@ -958,8 +957,7 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 	case L('!'):
 	  if (__builtin_expect (flags & FNM_EXTMATCH, 0) && *p == '(')
 	    {
-	      int res = EXT (c, p, n, string_end, no_leading_period, flags,
-			     alloca_used);
+	      int res = EXT (c, p, n, string_end, no_leading_period, flags);
 	      if (res != -1)
 		return res;
 	    }
@@ -1038,26 +1036,37 @@ END (const CHAR *pattern)
   return p + 1;
 }
 
+#if WIDE_CHAR_VERSION
+# define PATTERN_PREFIX pattern_list
+#else
+# define PATTERN_PREFIX wpattern_list
+#endif
+
+#define PASTE(a,b)                 PASTE1(a,b)
+#define PASTE1(a,b)                a##b
+
+#define DYNARRAY_STRUCT            PATTERN_PREFIX
+#define DYNARRAY_ELEMENT_FREE(ptr) free (*ptr)
+#define DYNARRAY_ELEMENT           CHAR *
+#define DYNARRAY_PREFIX            PASTE(PATTERN_PREFIX,_)
+#define DYNARRAY_INITIAL_SIZE      8
+#include <malloc/dynarray-skeleton.c>
 
 static int
 EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
-     int no_leading_period, int flags, size_t alloca_used)
+     int no_leading_period, int flags)
 {
   const CHAR *startp;
   int level;
-  struct patternlist
-  {
-    struct patternlist *next;
-    CHAR malloced;
-    CHAR str[0];
-  } *list = NULL;
-  struct patternlist **lastp = &list;
+  struct PATTERN_PREFIX list;
   size_t pattern_len = STRLEN (pattern);
-  int any_malloced = 0;
+  size_t pattern_i = 0;
   const CHAR *p;
   const CHAR *rs;
   int retval = 0;
 
+  PASTE (PATTERN_PREFIX, _init) (&list);
+
   /* Parse the pattern.  Store the individual parts in the list.  */
   level = 0;
   for (startp = p = pattern + 1; level >= 0; ++p)
@@ -1099,28 +1108,20 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 	  {
 	    /* This means we found the end of the pattern.  */
 #define NEW_PATTERN \
-	    struct patternlist *newp;					      \
-	    size_t slen = (opt == L('?') || opt == L('@')		      \
-			   ? pattern_len : (p - startp + 1));		      \
-	    slen = sizeof (struct patternlist) + (slen * sizeof (CHAR));      \
-	    int malloced = ! __libc_use_alloca (alloca_used + slen);	      \
-	    if (__builtin_expect (malloced, 0))				      \
-	      {								      \
-		newp = malloc (slen);					      \
-		if (newp == NULL)					      \
-		  {							      \
-		    retval = -2;					      \
-		    goto out;						      \
-		  }							      \
-		any_malloced = 1;					      \
-	      }								      \
-	    else							      \
-	      newp = alloca_account (slen, alloca_used);		      \
-	    newp->next = NULL;						      \
-	    newp->malloced = malloced;					      \
-	    *((CHAR *) MEMPCPY (newp->str, startp, p - startp)) = L('\0');    \
-	    *lastp = newp;						      \
-	    lastp = &newp->next
+	    size_t slen = (opt == L('?') || opt == L('@')		     \
+			   ? pattern_len : (p - startp + 1));		     \
+	    CHAR *newp = malloc (slen * sizeof (CHAR));			     \
+	    if (newp != NULL)						     \
+	      {								     \
+	        *((CHAR *) MEMPCPY (newp, startp, p - startp)) = L('\0');    \
+	        PASTE (PATTERN_PREFIX,_add) (&list, newp);		     \
+	      }								     \
+	    if (newp == NULL || PASTE (PATTERN_PREFIX, _has_failed) (&list)) \
+	      {								     \
+		retval = -2;						     \
+		goto out;						     \
+	      }								     \
+
 	    NEW_PATTERN;
 	  }
       }
@@ -1132,27 +1133,26 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 	    startp = p + 1;
 	  }
       }
-  assert (list != NULL);
   assert (p[-1] == L(')'));
 #undef NEW_PATTERN
 
   switch (opt)
     {
     case L('*'):
-      if (FCT (p, string, string_end, no_leading_period, flags, NULL,
-	       alloca_used) == 0)
+      if (FCT (p, string, string_end, no_leading_period, flags, NULL) == 0)
 	goto success;
       /* FALLTHROUGH */
 
     case L('+'):
-      do
+      for (; pattern_i < PASTE (PATTERN_PREFIX, _size)(&list); pattern_i++)
 	{
 	  for (rs = string; rs <= string_end; ++rs)
 	    /* First match the prefix with the current pattern with the
 	       current pattern.  */
-	    if (FCT (list->str, string, rs, no_leading_period,
+	    if (FCT (*PASTE (PATTERN_PREFIX, _at) (&list, pattern_i), string,
+		     rs, no_leading_period,
 		     flags & FNM_FILE_NAME ? flags : flags & ~FNM_PERIOD,
-		     NULL, alloca_used) == 0
+		     NULL) == 0
 		/* This was successful.  Now match the rest with the rest
 		   of the pattern.  */
 		&& (FCT (p, rs, string_end,
@@ -1160,7 +1160,7 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 			 ? no_leading_period
 			 : rs[-1] == '/' && NO_LEADING_PERIOD (flags) ? 1 : 0,
 			 flags & FNM_FILE_NAME
-			 ? flags : flags & ~FNM_PERIOD, NULL, alloca_used) == 0
+			 ? flags : flags & ~FNM_PERIOD, NULL) == 0
 		    /* This didn't work.  Try the whole pattern.  */
 		    || (rs != string
 			&& FCT (pattern - 1, rs, string_end,
@@ -1169,36 +1169,34 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 				: (rs[-1] == '/' && NO_LEADING_PERIOD (flags)
 				   ? 1 : 0),
 				flags & FNM_FILE_NAME
-				? flags : flags & ~FNM_PERIOD, NULL,
-				alloca_used) == 0)))
+				? flags : flags & ~FNM_PERIOD, NULL) == 0)))
 	      /* It worked.  Signal success.  */
 	      goto success;
 	}
-      while ((list = list->next) != NULL);
 
       /* None of the patterns lead to a match.  */
       retval = FNM_NOMATCH;
       break;
 
     case L('?'):
-      if (FCT (p, string, string_end, no_leading_period, flags, NULL,
-	       alloca_used) == 0)
+      if (FCT (p, string, string_end, no_leading_period, flags, NULL) == 0)
 	goto success;
       /* FALLTHROUGH */
 
     case L('@'):
-      do
-	/* I cannot believe it but `strcat' is actually acceptable
-	   here.  Match the entire string with the prefix from the
-	   pattern list and the rest of the pattern following the
-	   pattern list.  */
-	if (FCT (STRCAT (list->str, p), string, string_end,
-		 no_leading_period,
-		 flags & FNM_FILE_NAME ? flags : flags & ~FNM_PERIOD,
-		 NULL, alloca_used) == 0)
-	  /* It worked.  Signal success.  */
-	  goto success;
-      while ((list = list->next) != NULL);
+      for (; pattern_i < PASTE (PATTERN_PREFIX, _size) (&list); pattern_i++)
+	{
+	  /* I cannot believe it but `strcat' is actually acceptable
+	     here.  Match the entire string with the prefix from the
+	     pattern list and the rest of the pattern following the
+	     pattern list.  */
+	  if (FCT (STRCAT (*PASTE (PATTERN_PREFIX, _at) (&list, pattern_i), p),
+		   string, string_end, no_leading_period,
+		   flags & FNM_FILE_NAME ? flags : flags & ~FNM_PERIOD,
+		   NULL) == 0)
+	    /* It worked.  Signal success.  */
+	    goto success;
+	}
 
       /* None of the patterns lead to a match.  */
       retval = FNM_NOMATCH;
@@ -1207,22 +1205,27 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
     case L('!'):
       for (rs = string; rs <= string_end; ++rs)
 	{
-	  struct patternlist *runp;
+	  size_t runp_i;
 
-	  for (runp = list; runp != NULL; runp = runp->next)
-	    if (FCT (runp->str, string, rs,  no_leading_period,
-		     flags & FNM_FILE_NAME ? flags : flags & ~FNM_PERIOD,
-		     NULL, alloca_used) == 0)
+	  for (runp_i = pattern_i;
+	       runp_i != PASTE (PATTERN_PREFIX, _size) (&list);
+	       runp_i++)
+	    {
+	      if (FCT (*PASTE (PATTERN_PREFIX, _at) (&list, runp_i), string, rs,
+		       no_leading_period,
+		       flags & FNM_FILE_NAME ? flags : flags & ~FNM_PERIOD,
+		       NULL) == 0)
 	      break;
+	    }
 
 	  /* If none of the patterns matched see whether the rest does.  */
-	  if (runp == NULL
+	  if (runp_i == PASTE (PATTERN_PREFIX, _size) (&list)
 	      && (FCT (p, rs, string_end,
 		       rs == string
 		       ? no_leading_period
 		       : rs[-1] == '/' && NO_LEADING_PERIOD (flags) ? 1 : 0,
 		       flags & FNM_FILE_NAME ? flags : flags & ~FNM_PERIOD,
-		       NULL, alloca_used) == 0))
+		       NULL) == 0))
 	    /* This is successful.  */
 	    goto success;
 	}
@@ -1240,18 +1243,15 @@ EXT (INT opt, const CHAR *pattern, const CHAR *string, const CHAR *string_end,
 
  success:
  out:
-  if (any_malloced)
-    while (list != NULL)
-      {
-	struct patternlist *old = list;
-	list = list->next;
-	if (old->malloced)
-	  free (old);
-      }
+  PASTE (PATTERN_PREFIX, _free) (&list);
 
   return retval;
 }
 
+#undef PATTERN_PREFIX
+#undef PASTE
+#undef PASTE1
+
 
 #undef FOLD
 #undef CHAR
-- 
2.7.4

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

* [PATCH v2 07/12] posix: Use char_array for home_dir in glob
  2018-02-05 13:27 [PATCH v2 00/12] posix: glob/fnmatch fixes and refactor Adhemerval Zanella
                   ` (3 preceding siblings ...)
  2018-02-05 13:27 ` [PATCH v2 06/12] posix: Remove alloca usage on glob user_name Adhemerval Zanella
@ 2018-02-05 13:28 ` Adhemerval Zanella
  2018-02-05 13:28 ` [PATCH v2 01/12] malloc: Add specialized dynarray for C strings Adhemerval Zanella
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 13:28 UTC (permalink / raw)
  To: libc-alpha

This patch uses dynarray at glob internal home directory ame
manipulation for GLOB_TILDE.  It simplifies it and removes all the
boilerplate buffer managements required.

Checked x86_64-linux-gnu.

	* posix/glob.c (__glob): Use char_array for home directory.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 ChangeLog    |  2 ++
 posix/glob.c | 66 ++++++++++++++++++++++++++++++++++++------------------------
 2 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/posix/glob.c b/posix/glob.c
index e914748..a78718d 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -596,9 +596,14 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		  || char_array_pos (&dirname, 2) == '/')))
 	{
 	  /* Look up home directory.  */
-	  char *home_dir = getenv ("HOME");
-	  int malloc_home_dir = 0;
-	  if (home_dir == NULL || home_dir[0] == '\0')
+	  struct char_array home_dir;
+
+	  const char *home_env = getenv ("HOME");
+	  home_env = home_env == NULL ? "" : home_env;
+	  if (!char_array_init_str (&home_dir, home_env))
+	    goto err_nospace;
+
+	  if (char_array_is_empty (&home_dir))
 	    {
 #ifdef WINDOWS32
 	      /* Windows NT defines HOMEDRIVE and HOMEPATH.  But give
@@ -608,16 +613,21 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 
 	      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;
+		  if (!char_array_set_str (&home_dir, home_drive)
+		      || !char_array_append_str (&home_dir, home_path))
+		   {
+		     char_array_free (&home_dir);
+		     goto err_nospace;
+		   }
 		}
 	      else
-		home_dir = "c:/users/default"; /* poor default */
+		{
+		  if (!char_array_set_str (&home_dir, "c:/users/default"))
+		    {
+		      char_array_free (&home_dir);
+		      goto err_nospace;
+		    }
+		}
 #else
 	      int err;
 	      struct passwd *p;
@@ -646,44 +656,48 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		  if (!scratch_buffer_grow (&s))
 		    goto err_nospace;
 		}
-	      if (err == 0)
-		{
-		  home_dir = strdup (p->pw_dir);
-		  malloc_home_dir = 1;
-		}
+	      bool e = (err == 0 && char_array_set_str (&home_dir, p->pw_dir));
 	      scratch_buffer_free (&s);
-	      if (err == 0 && home_dir == NULL)
+	      if (e == false)
 		goto err_nospace;
 #endif /* WINDOWS32 */
 	    }
-	  if (home_dir == NULL || home_dir[0] == '\0')
+	  if (char_array_is_empty (&home_dir))
 	    {
-	      if (__glibc_unlikely (malloc_home_dir))
-		free (home_dir);
 	      if (flags & GLOB_TILDE_CHECK)
 		{
+		  char_array_free (&home_dir);
 		  retval = GLOB_NOMATCH;
 		  goto out;
 		}
 	      else
 		{
-		  home_dir = (char *) "~"; /* No luck.  */
-		  malloc_home_dir = 0;
+		  if (!char_array_set_str (&home_dir, "~"))
+		    {
+		      char_array_free (&home_dir);
+		      goto err_nospace;
+		    }
 		}
 	    }
 	  /* Now construct the full directory.  */
+	  bool e = true;
 	  if (char_array_pos (&dirname, 1) == '\0')
 	    {
-	      if (!char_array_set_str (&dirname, home_dir))
-		goto err_nospace;
+	      e = char_array_set_str (&dirname, char_array_str (&home_dir));
 	      dirlen = char_array_size (&dirname) - 1;
 	    }
 	  else
 	    {
 	      /* Replaces '~' by the obtained HOME dir.  */
 	      char_array_erase (&dirname, 0);
-	      if (!char_array_prepend_str (&dirname, home_dir))
-		goto err_nospace;
+	      e = char_array_prepend_str (&dirname,
+					  char_array_str (&home_dir));
+	    }
+	  if (e == false)
+	    {
+	      char_array_free (&dirname);
+	      char_array_free (&home_dir);
+	      goto err_nospace;
 	    }
 	  dirname_modified = true;
 	}
-- 
2.7.4

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

* [PATCH v2 04/12] posix: Remove alloca usage on glob dirname
  2018-02-05 13:27 [PATCH v2 00/12] posix: glob/fnmatch fixes and refactor Adhemerval Zanella
                   ` (7 preceding siblings ...)
  2018-02-05 13:28 ` [PATCH v2 11/12] posix: Remove alloca usage for internal fnmatch implementation Adhemerval Zanella
@ 2018-02-05 13:28 ` Adhemerval Zanella
  2018-02-05 13:28 ` [PATCH v2 12/12] posix: Remove VLA usage for internal fnmatch implementation Adhemerval Zanella
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 13:28 UTC (permalink / raw)
  To: libc-alpha

This patch replaces the alloca/malloc usage for dirname creation
by the char_array struct.

Checked on x86_64-linux-gnu.

	* posix/glob.c (glob_in_dir): Remove alloca usage for fullname.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 ChangeLog    |  2 ++
 posix/glob.c | 28 +++++++++-------------------
 2 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/posix/glob.c b/posix/glob.c
index 3e36eeb..26af331 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -1198,7 +1198,6 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 	     int (*errfunc) (const char *, int),
 	     glob_t *pglob, size_t alloca_used)
 {
-  size_t dirlen = strlen (directory);
   void *stream = NULL;
 # define GLOBNAMES_MEMBERS(nnames) \
     struct globnames *next; size_t count; char *name[nnames];
@@ -1230,32 +1229,23 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
     }
   else if (meta == GLOBPAT_NONE)
     {
-      size_t patlen = strlen (pattern);
-      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 (fullsize, alloca_used);
-      else
+      struct char_array fullname;
+
+      if (!char_array_init_str (&fullname, directory)
+	  || !char_array_append_str (&fullname, "/")
+	  || !char_array_append_str (&fullname, pattern))
 	{
-	  fullname = malloc (fullsize);
-	  if (fullname == NULL)
-	    return GLOB_NOSPACE;
+	  char_array_free (&fullname);
+	  return GLOB_NOSPACE;
 	}
 
-      mempcpy (mempcpy (mempcpy (fullname, directory, dirlen),
-			"/", 1),
-	       pattern, patlen + 1);
-      if (glob_lstat (pglob, flags, fullname) == 0
+      if (glob_lstat (pglob, flags, char_array_str (&fullname)) == 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;
 
-      if (__glibc_unlikely (!alloca_fullname))
-	free (fullname);
+      char_array_free (&fullname);
     }
   else
     {
-- 
2.7.4

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

* [PATCH v2 01/12] malloc: Add specialized dynarray for C strings
  2018-02-05 13:27 [PATCH v2 00/12] posix: glob/fnmatch fixes and refactor Adhemerval Zanella
                   ` (4 preceding siblings ...)
  2018-02-05 13:28 ` [PATCH v2 07/12] posix: Use char_array for home_dir in glob Adhemerval Zanella
@ 2018-02-05 13:28 ` Adhemerval Zanella
  2018-02-05 13:28 ` [PATCH v2 05/12] posix: Use dynarray for globname in glob Adhemerval Zanella
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 13:28 UTC (permalink / raw)
  To: libc-alpha

This patch adds an specialized dynarray to manage C strings using the
dynarray internal implementation.  It uses some private fields from
dynarray and thus it provided specific files to access and manage
the internal string buffer.

   For instance:

   struct char_array str;

   // str == "testing"
   char_array_init_str (&str, "testing");

   // c == 's'
   char c = char_array_pos (&str, 2);

   // str = "testing2"
   char_array_set_str (&str, "testing2");

   // str = "testig2"
   char_array_erase (&str, 5);

   // str = "123testig2";
   char_array_prepend_str (&str, "123");

   // len = 10;
   size_t len = char_array_length (&str);

   // str = "123testig2456";
   char_array_append_str (&str, "456");

   // str = "123test789";
   char_array_replace_str_pos (&str, 7, "789", 3);

The provided function are not extensive and meant mainly to be use in
subsequent glob implementation cleanup.  For internal object consistency
only the function provided by char_array.c should be used, including
internal object manipulation.

To check for possible overflows in internal size manipulation a new
function, check_add_wrapv_size_t, is added on malloc-internal.  It basically
return whether the addition of two size_t overflows.

Checked on x86_64-linux-gnu.

	* malloc/Makefile (test-internal): Add tst-char_array.
	(routines): Add dynarray_overflow_failure and char_array-impl.
	* malloc/Versions [GLIBC_PRIVATE] (libc): Add
	__libc_dynarray_overflow_failure, __char_array_set_str_size,
	__char_array_erase, __char_array_prepend_str_size, and
	__char_array_replace_str_pos.
	* malloc/char_array-impl.c: New file.
	* malloc/char_array-skeleton.c: Likewise.
	* malloc/char_array.h: Likewise.
	* malloc/tst-char-array.c: Likewise.
	* malloc/dynarray_overflow_failure.c: Likewise.
	* malloc/malloc-internal.h (check_add_overflow_size_t): New function.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 ChangeLog                          |  15 ++
 malloc/Makefile                    |   4 +-
 malloc/Versions                    |   7 +
 malloc/char_array-impl.c           |  57 ++++++++
 malloc/char_array-skeleton.c       | 288 +++++++++++++++++++++++++++++++++++++
 malloc/char_array.h                |  53 +++++++
 malloc/dynarray.h                  |   9 ++
 malloc/dynarray_overflow_failure.c |  31 ++++
 malloc/malloc-internal.h           |  14 ++
 malloc/tst-char_array.c            | 112 +++++++++++++++
 10 files changed, 589 insertions(+), 1 deletion(-)
 create mode 100644 malloc/char_array-impl.c
 create mode 100644 malloc/char_array-skeleton.c
 create mode 100644 malloc/char_array.h
 create mode 100644 malloc/dynarray_overflow_failure.c
 create mode 100644 malloc/tst-char_array.c

diff --git a/malloc/Makefile b/malloc/Makefile
index 17873e6..1b0b5c5 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -50,6 +50,7 @@ tests-internal += \
 	 tst-dynarray \
 	 tst-dynarray-fail \
 	 tst-dynarray-at-fail \
+	 tst-char_array
 
 ifneq (no,$(have-tunables))
 tests += tst-malloc-usable-tunables
@@ -62,7 +63,7 @@ test-srcs = tst-mtrace
 routines = malloc morecore mcheck mtrace obstack reallocarray \
   scratch_buffer_grow scratch_buffer_grow_preserve \
   scratch_buffer_set_array_size \
-  dynarray_at_failure \
+  dynarray_at_failure dynarray_overflow_failure \
   dynarray_emplace_enlarge \
   dynarray_finalize \
   dynarray_resize \
@@ -72,6 +73,7 @@ routines = malloc morecore mcheck mtrace obstack reallocarray \
   alloc_buffer_copy_bytes  \
   alloc_buffer_copy_string \
   alloc_buffer_create_failure \
+  char_array-impl
 
 install-lib := libmcheck.a
 non-lib.a := libmcheck.a
diff --git a/malloc/Versions b/malloc/Versions
index 2357cff..b21fe59 100644
--- a/malloc/Versions
+++ b/malloc/Versions
@@ -81,6 +81,7 @@ libc {
 
     # dynarray support
     __libc_dynarray_at_failure;
+    __libc_dynarray_overflow_failure;
     __libc_dynarray_emplace_enlarge;
     __libc_dynarray_finalize;
     __libc_dynarray_resize;
@@ -92,5 +93,11 @@ libc {
     __libc_alloc_buffer_copy_bytes;
     __libc_alloc_buffer_copy_string;
     __libc_alloc_buffer_create_failure;
+
+    # char_array support
+    __char_array_set_str_size;
+    __char_array_erase;
+    __char_array_prepend_str_size;
+    __char_array_replace_str_pos;
   }
 }
diff --git a/malloc/char_array-impl.c b/malloc/char_array-impl.c
new file mode 100644
index 0000000..3aef1f7
--- /dev/null
+++ b/malloc/char_array-impl.c
@@ -0,0 +1,57 @@
+/* Specialized dynarray for C strings.  Implementation file.
+   Copyright (C) 2018 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 <malloc/char_array.h>
+
+void
+__char_array_set_str_size (struct dynarray_header *header, const char *str,
+			   size_t size)
+{
+  *((char *) mempcpy (header->array, str, size)) = '\0';
+  header->used = size + 1;
+}
+libc_hidden_def (__char_array_set_str_size)
+
+void
+__char_array_erase (struct dynarray_header *header, size_t pos)
+{
+  char *ppos = header->array + pos;
+  char *lpos = header->array + header->used;
+  ptrdiff_t size = lpos - ppos;
+  memmove (ppos, ppos + 1, size);
+  header->used--;
+}
+libc_hidden_def (__char_array_erase)
+
+void
+__char_array_prepend_str_size (struct dynarray_header *header,
+			       const char *str, size_t size, size_t used)
+{
+  memmove (header->array + size, header->array, used);
+  memcpy (header->array, str, size);
+}
+libc_hidden_def (__char_array_prepend_str_size)
+
+void
+__char_array_replace_str_pos (struct dynarray_header *header, size_t pos,
+			      const char *str, size_t len)
+{
+  char *start = header->array + pos;
+  *(char *) mempcpy (start, str, len) = '\0';
+}
+libc_hidden_def (__char_array_replace_str_pos)
diff --git a/malloc/char_array-skeleton.c b/malloc/char_array-skeleton.c
new file mode 100644
index 0000000..4d9e4c0
--- /dev/null
+++ b/malloc/char_array-skeleton.c
@@ -0,0 +1,288 @@
+/* Specialized dynarray for C strings.
+   Copyright (C) 2018 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/>.  */
+
+/* This file provides a dynamic C string with an initial stack allocated
+   buffer.  Since it is based on dynarray, it provides dynamic size
+   expansion and heap usage for large strings.
+
+   The following parameters are optional:
+
+   CHAR_ARRAY_INITIAL_SIZE
+      The size of the statically allocated array (default is 256).  It will
+      be used to define DYNARRAY_INITIAL_SIZE.
+
+   The following functions are provided:
+
+   bool char_array_init_empty (struct char_array *);
+   bool char_array_init_str (struct char_array *, const char *);
+   bool char_array_init_str_size (struct char_array *, const char *, size_t);
+   bool char_array_is_empty (struct char_array *);
+   const char *char_array_str (struct char_array *);
+   char char_array_pos (struct char_array *, size_t);
+   size_t char_array_length (struct char_array *);
+   bool char_array_set_str (struct char_array *, const char *);
+   bool char_array_set_str_size (struct char_array *, const char *, size_t);
+   void char_array_erase (struct char_array *, size_t);
+   bool char_array_crop (struct char_array *, size_t);
+   bool char_array_prepend_str (struct char_array *, const char *);
+   bool char_array_append_str (struct char_array *, const char *);
+   bool char_array_append_char (struct char_array *array, char c);
+   bool char_array_replace_str_pos (struct char_array *, size_t, const char *,
+				    size_t);
+
+   For instance:
+
+   struct char_array str;
+
+   // str == "testing"
+   char_array_init_str (&str, "testing");
+
+   // c == 's'
+   char c = char_array_pos (&str, 2);
+
+   // str = "testing2"
+   char_array_set_str (&str, "testing2");
+
+   // str = "testig2"
+   char_array_erase (&str, 5);
+
+   // str = "123testig2";
+   char_array_prepend_str (&str, "123");
+
+   // len = 10;
+   size_t len = char_array_length (&str);
+
+   // str = "123testig2456";
+   char_array_append_str (&str, "456");
+
+   // str = "123test789";
+   char_array_replace_str_pos (&str, 7, "789", 3);
+ */
+
+#define DYNARRAY_STRUCT            char_array
+#define DYNARRAY_ELEMENT           char
+#define DYNARRAY_PREFIX            char_array_
+#ifndef CHAR_ARRAY_INITIAL_SIZE
+# define CHAR_ARRAY_INITIAL_SIZE 256
+#endif
+#define DYNARRAY_INITIAL_SIZE  CHAR_ARRAY_INITIAL_SIZE
+#include <malloc/dynarray-skeleton.c>
+
+#include <malloc/malloc-internal.h>
+#include <malloc/char_array.h>
+
+/* Return a const char for the internal C string handled by 'array'.  */
+__attribute__ ((unused, nonnull (1)))
+static const char *
+char_array_str (const struct char_array *array)
+{
+  return array->dynarray_header.array;
+}
+
+/* Return the character at position 'pos' from the char_array 'array'.  */
+__attribute__ ((unused, nonnull (1)))
+static char
+char_array_pos (struct char_array *array, size_t pos)
+{
+  return *char_array_at (array, pos);
+}
+
+/* Calculate the length of the string, excluding the terminating null.  */
+__attribute__ ((unused, nonnull (1)))
+static size_t
+char_array_length (const struct char_array *array)
+{
+  /* Exclude the final '\0'.  */
+  return array->dynarray_header.used - 1;
+}
+
+/* Copy up 'size' bytes from string 'str' to char_array 'array'.  A final
+   '\0' is appended in the char_array.  */
+__attribute__ ((unused, nonnull (1, 2)))
+static bool
+char_array_set_str_size (struct char_array *array, const char *str,
+			 size_t size)
+{
+  size_t newsize;
+  if (check_add_overflow_size_t (size, 1, &newsize))
+    __libc_dynarray_overflow_failure (size, 1);
+
+  if (!char_array_resize (array, newsize))
+    return false;
+
+  __char_array_set_str_size (&array->dynarray_abstract, str, size);
+  return true;
+}
+
+/* Copy the contents of string 'str' to char_array 'array', including the
+   final '\0'.  */
+__attribute__ ((unused, nonnull (1, 2)))
+static bool
+char_array_set_str (struct char_array *array, const char *str)
+{
+  return char_array_set_str_size (array, str, strlen (str));
+}
+
+__attribute__ ((unused, nonnull (1, 2)))
+static bool
+char_array_set_array (struct char_array *array, const struct char_array *orig)
+{
+  return char_array_set_str_size (array, char_array_str (orig),
+				  char_array_length (orig));
+}
+
+/* Initialize the char_array 'array' and sets it to an empty string ("").  */
+__attribute__ ((unused, nonnull (1)))
+static bool
+char_array_init_empty (struct char_array *array)
+{
+  char_array_init (array);
+  return char_array_set_str (array, "");
+}
+
+/* Initialize the char_array 'array' and copy the content of string 'str'.  */
+__attribute__ ((unused, nonnull (1, 2)))
+static bool
+char_array_init_str (struct char_array *array, const char *str)
+{
+  char_array_init (array);
+  return char_array_set_str (array, str);
+}
+
+/* Initialize the char_array 'array' and copy the content of string 'str'
+   up to 'size' characteres.  */
+__attribute__ ((unused, nonnull (1, 2)))
+static bool
+char_array_init_str_size (struct char_array *array, const char *str,
+			  size_t size)
+{
+  char_array_init (array);
+  return char_array_set_str_size (array, str, size);
+}
+
+/* Return if the char_array contain any characteres.  */
+__attribute__ ((unused, nonnull (1)))
+static bool
+char_array_is_empty (struct char_array *array)
+{
+  return *char_array_begin (array) == '\0';
+}
+
+/* Remove the byte at position 'pos' from char_array 'array'.  The contents
+   are moved internally if the position is not at the end of the internal
+   buffer.  */
+__attribute__ ((unused, nonnull (1)))
+static bool
+char_array_erase (struct char_array *array, size_t pos)
+{
+  if (pos >= array->dynarray_header.used - 1)
+    return false;
+
+  __char_array_erase (&array->dynarray_abstract, pos);
+  return true;
+}
+
+/* Resize the char_array 'array' to size 'count' maintaining the ending
+   '\0' byte.  */
+__attribute__ ((unused, nonnull (1)))
+static bool
+char_array_crop (struct char_array *array, size_t size)
+{
+  if (size >= (array->dynarray_header.used - 1)
+      || !char_array_resize (array, size + 1))
+    return false;
+
+  array->dynarray_header.array[size] = '\0';
+  return true;
+}
+
+/* Prepend the contents of string 'str' to char_array 'array', including the
+   final '\0' byte.  */
+__attribute__ ((unused, nonnull (1, 2)))
+static bool
+char_array_prepend_str (struct char_array *array, const char *str)
+{
+  size_t size = strlen (str);
+  /* Resizing the array might change its used elements and we need below
+     to correct copy the elements.  */
+  size_t used = array->dynarray_header.used;
+
+  size_t newsize;
+  if (check_add_overflow_size_t (used, size, &newsize))
+    __libc_dynarray_overflow_failure (used, size);
+
+  /* Make room for the string and copy it.  */
+  if (!char_array_resize (array, newsize))
+    return false;
+  __char_array_prepend_str_size (&array->dynarray_abstract, str, size, used);
+  return true;
+}
+
+/* Append the contents of string 'str' to char_array 'array, including the
+   final '\0' byte.  */
+__attribute__ ((unused, nonnull (1, 2)))
+static bool
+char_array_append_str (struct char_array *array, const char *str)
+{
+  size_t size = strlen (str);
+  /* Resizing the array might change its used elements and it used it below
+     to correct copy the elements.  */
+  size_t used = array->dynarray_header.used - 1;
+
+  /* 'used' does account for final '\0', so there is no need to add
+     an extra element to calculate the final required size.  */
+  size_t newsize;
+  if (check_add_overflow_size_t (used + 1, size, &newsize))
+    __libc_dynarray_overflow_failure (used + 1, size);
+
+  if (!char_array_resize (array, newsize))
+    return false;
+
+  /* Start to append at '\0' up to string length and add a final '\0'.  */
+  *(char*) mempcpy (array->dynarray_header.array + used, str, size) = '\0';
+  return true;
+}
+
+/* Append the character 'c' on char_array 'array'.  */
+__attribute__ ((unused, nonnull (1)))
+static bool
+char_array_append_char (struct char_array *array, char c)
+{
+  return char_array_append_str (array, (const char[]) { c, '\0' });
+}
+
+/* Replace the contents starting of position 'pos' of char_array 'array'
+   with the contents of string 'str' up to 'len' bytes.  A final '\0'
+   is appended in the string.  */
+__attribute__ ((unused, nonnull (1, 3)))
+static bool
+char_array_replace_str_pos (struct char_array *array, size_t pos,
+                            const char *str, size_t len)
+{
+  if (pos > array->dynarray_header.used)
+    __libc_dynarray_at_failure (array->dynarray_header.used, pos);
+
+  size_t newsize;
+  if (check_add_overflow_size_t (pos, len, &newsize)
+      || check_add_overflow_size_t (newsize, 1, &newsize)
+      || !char_array_resize (array, newsize))
+    return false;
+
+  __char_array_replace_str_pos (&array->dynarray_abstract, pos, str, len);
+  return true;
+}
diff --git a/malloc/char_array.h b/malloc/char_array.h
new file mode 100644
index 0000000..93df0d9
--- /dev/null
+++ b/malloc/char_array.h
@@ -0,0 +1,53 @@
+/* Specialized dynarray for C strings.  Shared definitions.
+   Copyright (C) 2018 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 _CHAR_ARRAY_H
+#define _CHAR_ARRAY_H
+
+#include <malloc/dynarray.h>
+
+/* Internal funciton.  Set the dynarray to the content of the string STR up
+   to SIZE bytes.  The dynarray must be resized previously.  */
+void __char_array_set_str_size (struct dynarray_header *, const char *str,
+				size_t size);
+
+/* Internal function.  Remove the character at position POS from dynarray.
+   The position must be a valid one.  */
+void __char_array_erase (struct dynarray_header *, size_t pos);
+
+/* Internal function.  Prepend the content of string STR up to SIZE bytes to
+   dynarray by moving USED bytes forward.  The dynarray must be resized
+   previously.  */
+void __char_array_prepend_str_size (struct dynarray_header *,
+				    const char *str, size_t size,
+				    size_t used);
+
+/* Internal function.  Replace the content of dynarray starting at position
+   POS with the content of string STR up to LEN bytes.  The dynarray must
+   be resize previously and STR must contain at least LEN bytes.  */
+void __char_array_replace_str_pos (struct dynarray_header *, size_t pos,
+				   const char *str, size_t len);
+
+#ifndef _ISOMAC
+libc_hidden_proto (__char_array_set_str_size)
+libc_hidden_proto (__char_array_erase)
+libc_hidden_proto (__char_array_prepend_str_size)
+libc_hidden_proto (__char_array_replace_str_pos)
+#endif
+
+#endif
diff --git a/malloc/dynarray.h b/malloc/dynarray.h
index 0b171da..fd8d218 100644
--- a/malloc/dynarray.h
+++ b/malloc/dynarray.h
@@ -168,12 +168,21 @@ bool __libc_dynarray_finalize (struct dynarray_header *list, void *scratch,
 void __libc_dynarray_at_failure (size_t size, size_t index)
   __attribute__ ((noreturn));
 
+/* Internal function.  Terminate the process after an overflow in
+   new size allocation.  SIZE is the current number of elements in
+   dynamic array and INCR is the new elements to add on current
+   size.  */
+void __libc_dynarray_overflow_failure (size_t size, size_t incr)
+  __attribute__ ((noreturn));
+
 #ifndef _ISOMAC
 libc_hidden_proto (__libc_dynarray_emplace_enlarge)
 libc_hidden_proto (__libc_dynarray_resize)
 libc_hidden_proto (__libc_dynarray_resize_clear)
 libc_hidden_proto (__libc_dynarray_finalize)
 libc_hidden_proto (__libc_dynarray_at_failure)
+libc_hidden_proto (__libc_dynarray_overflow_failure)
+
 #endif
 
 #endif /* _DYNARRAY_H */
diff --git a/malloc/dynarray_overflow_failure.c b/malloc/dynarray_overflow_failure.c
new file mode 100644
index 0000000..2bcef25
--- /dev/null
+++ b/malloc/dynarray_overflow_failure.c
@@ -0,0 +1,31 @@
+/* Report an dynamic array size overflow condition.
+   Copyright (C) 2018 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 <dynarray.h>
+#include <stdio.h>
+
+void
+__libc_dynarray_overflow_failure (size_t size, size_t incr)
+{
+  char buf[200];
+  __snprintf (buf, sizeof (buf), "Fatal glibc error: "
+              "new size overflows (old %zu and increment %zu)\n",
+              size, incr);
+ __libc_fatal (buf);
+}
+libc_hidden_def (__libc_dynarray_overflow_failure)
diff --git a/malloc/malloc-internal.h b/malloc/malloc-internal.h
index ad05450..352db0bf 100644
--- a/malloc/malloc-internal.h
+++ b/malloc/malloc-internal.h
@@ -91,4 +91,18 @@ check_mul_overflow_size_t (size_t left, size_t right, size_t *result)
 #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.  */
+static inline bool
+check_add_overflow_size_t (size_t a, size_t b, size_t *r)
+{
+#if __GNUC__ >= 5
+  return __builtin_add_overflow (a, b, r);
+#else
+  *r = a + b;
+  return *r < a;
+#endif
+}
+
 #endif /* _MALLOC_INTERNAL_H */
diff --git a/malloc/tst-char_array.c b/malloc/tst-char_array.c
new file mode 100644
index 0000000..745ecf4
--- /dev/null
+++ b/malloc/tst-char_array.c
@@ -0,0 +1,112 @@
+/* Test for char_array.
+   Copyright (C) 2018 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 <string.h>
+
+#include <malloc/char_array-skeleton.c>
+
+#include <malloc.h>
+#include <mcheck.h>
+#include <stdint.h>
+#include <support/check.h>
+#include <support/support.h>
+
+static int
+do_test (void)
+{
+  mtrace ();
+
+  {
+    struct char_array str;
+    TEST_VERIFY_EXIT (char_array_init_empty (&str) == true);
+    TEST_VERIFY_EXIT (char_array_length (&str) == 0);
+    TEST_VERIFY_EXIT (char_array_is_empty (&str) == true);
+    TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "") == 0);
+    char_array_free (&str);
+  }
+
+  {
+    struct char_array str;
+    TEST_VERIFY_EXIT (char_array_init_str (&str, "testing"));
+    TEST_VERIFY_EXIT (char_array_length (&str) == strlen ("testing"));
+    TEST_VERIFY_EXIT (char_array_pos (&str, 2) == 's');
+    TEST_VERIFY_EXIT (char_array_is_empty (&str) == false);
+    TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "testing") == 0);
+    char_array_free (&str);
+  }
+
+  {
+    struct char_array str;
+    TEST_VERIFY_EXIT (char_array_init_str_size (&str, "testing", 4));
+    TEST_VERIFY_EXIT (char_array_length (&str) == 4);
+    TEST_VERIFY_EXIT (char_array_pos (&str, 2) == 's');
+    TEST_VERIFY_EXIT (char_array_is_empty (&str) == false);
+    TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "test") == 0);
+    char_array_free (&str);
+  }
+
+  {
+    struct char_array str;
+    TEST_VERIFY_EXIT (char_array_init_str (&str, "testing"));
+    TEST_VERIFY_EXIT (char_array_set_str (&str, "abcdef"));
+    TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "abcdef") == 0);
+    TEST_VERIFY_EXIT (char_array_set_str_size (&str, "abcdef", 4));
+    TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "abcd") == 0);
+    char_array_free (&str);
+  }
+
+  {
+    struct char_array str;
+    TEST_VERIFY_EXIT (char_array_init_str (&str, "testing"));
+    TEST_VERIFY_EXIT (char_array_erase (&str, 4) == true);
+    TEST_VERIFY_EXIT (char_array_length (&str) == strlen ("testing") - 1);
+    TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "testng") == 0);
+    TEST_VERIFY_EXIT (char_array_erase (&str, char_array_length (&str))
+		      == false);
+    TEST_VERIFY_EXIT (char_array_length (&str) == strlen ("testing") - 1);
+    TEST_VERIFY_EXIT (char_array_erase (&str, char_array_length (&str) - 1)
+		      == true);
+    TEST_VERIFY_EXIT (char_array_length (&str) == strlen ("testing") - 2);
+    TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "testn") == 0);
+    char_array_free (&str);
+  }
+
+  {
+    struct char_array str;
+    TEST_VERIFY_EXIT (char_array_init_str (&str, "test"));
+    TEST_VERIFY_EXIT (char_array_prepend_str (&str, "123"));
+    TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "123test") == 0);
+    TEST_VERIFY_EXIT (char_array_length (&str) == strlen ("123test"));
+    TEST_VERIFY_EXIT (char_array_append_str (&str, "456"));
+    TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "123test456") == 0);
+    TEST_VERIFY_EXIT (char_array_length (&str) == strlen ("123test456"));
+    TEST_VERIFY_EXIT (char_array_replace_str_pos (&str, 7, "789", 3));
+    TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "123test789") == 0);
+    TEST_VERIFY_EXIT (char_array_length (&str) == strlen ("123test789"));
+    TEST_VERIFY_EXIT (char_array_crop (&str, 7));
+    TEST_VERIFY_EXIT (char_array_length (&str) == 7);
+    TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "123test") == 0);
+    TEST_VERIFY_EXIT (char_array_append_char (&str, '4'));
+    TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "123test4") == 0);
+    char_array_free (&str);
+  }
+
+  return 0;
+}
+
+#include <support/test-driver.c>
-- 
2.7.4

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

* [PATCH v2 02/12] posix: Use char_array for internal glob dirname
  2018-02-05 13:27 [PATCH v2 00/12] posix: glob/fnmatch fixes and refactor Adhemerval Zanella
                   ` (10 preceding siblings ...)
  2018-02-05 13:28 ` [PATCH v2 10/12] posix: Replace alloca usage with scratch_buffer for fnmatch Adhemerval Zanella
@ 2018-02-05 13:40 ` Adhemerval Zanella
  11 siblings, 0 replies; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 13:40 UTC (permalink / raw)
  To: libc-alpha

This is the first patch of the set to remove alloca usage on glob
implementation.  Internal path to search for file might expand to a
non static directory derived from pattern for some difference cases
(GLOB_NOESCAPE, GNU GLOB_TILDE) and to allow a non-static dirname
path glob uses a lot of boilerplate code to manage the buffer (which
is either allocated using alloca or malloc depending both to size
requested and the total alloca_used).

The patch changes to use the char_array struct with the default size
(256 bytes).  It simplifies all the allocation code by using char_array
one and every internal buffer access is done using char_array provided
functions.  No functional changes are expected.

Checked on x86_64-linux-gnu.

	* posix/globc.c (glob): Use char_array for dirname.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 ChangeLog    |   2 +
 posix/glob.c | 273 ++++++++++++++++++++++++-----------------------------------
 2 files changed, 112 insertions(+), 163 deletions(-)

diff --git a/posix/glob.c b/posix/glob.c
index 8444b2f..4814d67 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -77,6 +77,7 @@
 #include <flexmember.h>
 #include <glob_internal.h>
 #include <scratch_buffer.h>
+#include <malloc/char_array-skeleton.c>
 \f
 static const char *next_brace_sub (const char *begin, int flags) __THROWNL;
 
@@ -288,16 +289,15 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	glob_t *pglob)
 {
   const char *filename;
-  char *dirname = NULL;
   size_t dirlen;
   int status;
   size_t oldcount;
   int meta;
-  int dirname_modified;
-  int malloc_dirname = 0;
+  bool dirname_modified;
   glob_t dirs;
   int retval = 0;
   size_t alloca_used = 0;
+  struct char_array dirname;
 
   if (pattern == NULL || pglob == NULL || (flags & ~__GLOB_FLAGS) != 0)
     {
@@ -305,6 +305,10 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
       return -1;
     }
 
+  /* Default char array is stack allocated, so there is no need to check
+     if setting the initial '\0' succeeds.  */
+  char_array_init_empty (&dirname);
+
   /* POSIX requires all slashes to be matched.  This means that with
      a trailing slash we must match only directories.  */
   if (pattern[0] && pattern[strlen (pattern) - 1] == '/')
@@ -325,12 +329,12 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  size_t i;
 
 	  if (pglob->gl_offs >= ~((size_t) 0) / sizeof (char *))
-	    return GLOB_NOSPACE;
+	    goto err_nospace;
 
 	  pglob->gl_pathv = (char **) malloc ((pglob->gl_offs + 1)
 					      * sizeof (char *));
 	  if (pglob->gl_pathv == NULL)
-	    return GLOB_NOSPACE;
+	    goto err_nospace;
 
 	  for (i = 0; i <= pglob->gl_offs; ++i)
 	    pglob->gl_pathv[i] = NULL;
@@ -382,7 +386,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	    {
 	      onealt = malloc (pattern_len);
 	      if (onealt == NULL)
-		return GLOB_NOSPACE;
+		goto err_nospace;
 	    }
 
 	  /* We know the prefix for all sub-patterns.  */
@@ -444,7 +448,8 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		      globfree (pglob);
 		      pglob->gl_pathc = 0;
 		    }
-		  return result;
+		  retval = result;
+		  goto out;
 		}
 
 	      if (*next == '}')
@@ -461,9 +466,10 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 
 	  if (pglob->gl_pathc != firstc)
 	    /* We found some entries.  */
-	    return 0;
+	    retval = 0;
 	  else if (!(flags & (GLOB_NOCHECK|GLOB_NOMAGIC)))
-	    return GLOB_NOMATCH;
+	    retval = GLOB_NOMATCH;
+	  goto out;
 	}
     }
 
@@ -482,14 +488,15 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
     filename = strchr (pattern, ':');
 #endif /* __MSDOS__ || WINDOWS32 */
 
-  dirname_modified = 0;
+  dirname_modified = false;
   if (filename == NULL)
     {
       /* This can mean two things: a simple name or "~name".  The latter
 	 case is nothing but a notation for a directory.  */
       if ((flags & (GLOB_TILDE|GLOB_TILDE_CHECK)) && pattern[0] == '~')
 	{
-	  dirname = (char *) pattern;
+	  if (!char_array_set_str (&dirname, pattern))
+	    goto err_nospace;
 	  dirlen = strlen (pattern);
 
 	  /* Set FILENAME to NULL as a special flag.  This is ugly but
@@ -506,7 +513,8 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	    }
 
 	  filename = pattern;
-	  dirname = (char *) ".";
+	  if (!char_array_set_str (&dirname, "."))
+	    goto err_nospace;
 	  dirlen = 0;
 	}
     }
@@ -515,13 +523,13 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	       && (flags & GLOB_NOESCAPE) == 0))
     {
       /* "/pattern" or "\\/pattern".  */
-      dirname = (char *) "/";
+      if (!char_array_set_str (&dirname, "/"))
+	goto err_nospace;
       dirlen = 1;
       ++filename;
     }
   else
     {
-      char *newp;
       dirlen = filename - pattern;
 #if defined __MSDOS__ || defined WINDOWS32
       if (*filename == ':'
@@ -535,31 +543,25 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  /* 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;
+	    {
+	      retval = GLOB_NOMATCH;
+	      goto out;
+	    }
 	  /* 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
-
-      if (glob_use_alloca (alloca_used, dirlen + 1))
-	newp = alloca_account (dirlen + 1, alloca_used);
-      else
-	{
-	  newp = malloc (dirlen + 1);
-	  if (newp == NULL)
-	    return GLOB_NOSPACE;
-	  malloc_dirname = 1;
-	}
-      *((char *) mempcpy (newp, pattern, dirlen)) = '\0';
-      dirname = newp;
+      if (!char_array_set_str_size (&dirname, pattern, dirlen))
+	goto err_nospace;
       ++filename;
 
 #if defined __MSDOS__ || defined WINDOWS32
       bool drive_root = (dirlen > 1
-                         && (dirname[dirlen - 1] == ':'
-                             || (dirlen > 2 && dirname[dirlen - 2] == ':'
-                                 && dirname[dirlen - 1] == '/')));
+                         && (char_array_pos (&dirname, dirlen - 1) != ':'
+                             || (dirlen > 2
+				 && char_array_pos (&dirname, dirlen - 2) != ':'
+                                 && char_array_pos (&dirname, dirlen - 1) != '/')));
 #else
       bool drive_root = false;
 #endif
@@ -568,20 +570,24 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
         /* "pattern/".  Expand "pattern", appending slashes.  */
 	{
 	  int orig_flags = flags;
-	  if (!(flags & GLOB_NOESCAPE) && dirname[dirlen - 1] == '\\')
+	  if (!(flags & GLOB_NOESCAPE)
+	      && char_array_pos (&dirname, dirlen - 1) == '\\')
 	    {
 	      /* "pattern\\/".  Remove the final backslash if it hasn't
 		 been quoted.  */
-	      char *p = (char *) &dirname[dirlen - 1];
-
-	      while (p > dirname && p[-1] == '\\') --p;
-	      if ((&dirname[dirlen] - p) & 1)
+	      size_t p = dirlen - 1;
+	      while (p > 0 && char_array_pos (&dirname, p - 1) == '\\') --p;
+	      if ((dirlen - p) & 1)
 		{
-		  *(char *) &dirname[--dirlen] = '\0';
+		  /* Since we are shrinking the array, there is no need to
+		     check the function return.  */
+		  dirlen -= 1;
+		  char_array_crop (&dirname, dirlen);
 		  flags &= ~(GLOB_NOCHECK | GLOB_NOMAGIC);
 		}
 	    }
-	  int val = __glob (dirname, flags | GLOB_MARK, errfunc, pglob);
+	  int val = __glob (char_array_str (&dirname), flags | GLOB_MARK,
+			    errfunc, pglob);
 	  if (val == 0)
 	    pglob->gl_flags = ((pglob->gl_flags & ~GLOB_MARK)
 			       | (flags & GLOB_MARK));
@@ -598,11 +604,14 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	}
     }
 
-  if ((flags & (GLOB_TILDE|GLOB_TILDE_CHECK)) && dirname[0] == '~')
+  if ((flags & (GLOB_TILDE|GLOB_TILDE_CHECK))
+      && char_array_pos (&dirname, 0) == '~')
     {
-      if (dirname[1] == '\0' || dirname[1] == '/'
-	  || (!(flags & GLOB_NOESCAPE) && dirname[1] == '\\'
-	      && (dirname[2] == '\0' || dirname[2] == '/')))
+      if (char_array_pos (&dirname, 1) == '\0'
+	  || char_array_pos (&dirname, 1) == '/'
+	  || (!(flags & GLOB_NOESCAPE) && char_array_pos (&dirname, 1) == '\\'
+	      && (char_array_pos (&dirname, 2) == '\0'
+		  || char_array_pos (&dirname, 2) == '/')))
 	{
 	  /* Look up home directory.  */
 	  char *home_dir = getenv ("HOME");
@@ -653,10 +662,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		  if (err != ERANGE)
 		    break;
 		  if (!scratch_buffer_grow (&s))
-		    {
-		      retval = GLOB_NOSPACE;
-		      goto out;
-		    }
+		    goto err_nospace;
 		}
 	      if (err == 0)
 		{
@@ -665,10 +671,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		}
 	      scratch_buffer_free (&s);
 	      if (err == 0 && home_dir == NULL)
-		{
-		  retval = GLOB_NOSPACE;
-		  goto out;
-		}
+		goto err_nospace;
 #endif /* WINDOWS32 */
 	    }
 	  if (home_dir == NULL || home_dir[0] == '\0')
@@ -687,53 +690,26 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		}
 	    }
 	  /* Now construct the full directory.  */
-	  if (dirname[1] == '\0')
+	  if (char_array_pos (&dirname, 1) == '\0')
 	    {
-	      if (__glibc_unlikely (malloc_dirname))
-		free (dirname);
-
-	      dirname = home_dir;
-	      dirlen = strlen (dirname);
-	      malloc_dirname = malloc_home_dir;
+	      if (!char_array_set_str (&dirname, home_dir))
+		goto err_nospace;
+	      dirlen = char_array_size (&dirname) - 1;
 	    }
 	  else
 	    {
-	      char *newp;
-	      size_t home_len = strlen (home_dir);
-	      int use_alloca = glob_use_alloca (alloca_used, home_len + dirlen);
-	      if (use_alloca)
-		newp = alloca_account (home_len + dirlen, alloca_used);
-	      else
-		{
-		  newp = malloc (home_len + dirlen);
-		  if (newp == NULL)
-		    {
-		      if (__glibc_unlikely (malloc_home_dir))
-			free (home_dir);
-		      retval = GLOB_NOSPACE;
-		      goto out;
-		    }
-		}
-
-	      mempcpy (mempcpy (newp, home_dir, home_len),
-		       &dirname[1], dirlen);
-
-	      if (__glibc_unlikely (malloc_dirname))
-		free (dirname);
-
-	      dirname = newp;
-	      dirlen += home_len - 1;
-	      malloc_dirname = !use_alloca;
-
-	      if (__glibc_unlikely (malloc_home_dir))
-		free (home_dir);
+	      /* Replaces '~' by the obtained HOME dir.  */
+	      char_array_erase (&dirname, 0);
+	      if (!char_array_prepend_str (&dirname, home_dir))
+		goto err_nospace;
 	    }
-	  dirname_modified = 1;
+	  dirname_modified = true;
 	}
       else
 	{
 #ifndef WINDOWS32
-	  char *end_name = strchr (dirname, '/');
+	  char *dirnamestr = char_array_at (&dirname, 0);
+	  char *end_name = strchr (dirnamestr, '/');
 	  char *user_name;
 	  int malloc_user_name = 0;
 	  char *unescape = NULL;
@@ -742,23 +718,23 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	    {
 	      if (end_name == NULL)
 		{
-		  unescape = strchr (dirname, '\\');
+		  unescape = strchr (dirnamestr, '\\');
 		  if (unescape)
 		    end_name = strchr (unescape, '\0');
 		}
 	      else
-		unescape = memchr (dirname, '\\', end_name - dirname);
+		unescape = memchr (dirnamestr, '\\', end_name - dirnamestr);
 	    }
 	  if (end_name == NULL)
-	    user_name = dirname + 1;
+	    user_name = dirnamestr + 1;
 	  else
 	    {
 	      char *newp;
-	      if (glob_use_alloca (alloca_used, end_name - dirname))
-		newp = alloca_account (end_name - dirname, alloca_used);
+	      if (glob_use_alloca (alloca_used, end_name - dirnamestr))
+		newp = alloca_account (end_name - dirnamestr, alloca_used);
 	      else
 		{
-		  newp = malloc (end_name - dirname);
+		  newp = malloc (end_name - dirnamestr);
 		  if (newp == NULL)
 		    {
 		      retval = GLOB_NOSPACE;
@@ -768,8 +744,8 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		}
 	      if (unescape != NULL)
 		{
-		  char *p = mempcpy (newp, dirname + 1,
-				     unescape - dirname - 1);
+		  char *p = mempcpy (newp, dirnamestr + 1,
+				     unescape - dirnamestr - 1);
 		  char *q = unescape;
 		  while (q != end_name)
 		    {
@@ -791,7 +767,8 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		  *p = '\0';
 		}
 	      else
-		*((char *) mempcpy (newp, dirname + 1, end_name - dirname - 1))
+		*((char *) mempcpy (newp, dirnamestr + 1,
+				    end_name - dirnamestr - 1))
 		  = '\0';
 	      user_name = newp;
 	    }
@@ -825,35 +802,13 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	    /* If we found a home directory use this.  */
 	    if (p != NULL)
 	      {
-		size_t home_len = strlen (p->pw_dir);
-		size_t rest_len = end_name == NULL ? 0 : strlen (end_name);
-		char *d;
-
-		if (__glibc_unlikely (malloc_dirname))
-		  free (dirname);
-		malloc_dirname = 0;
-
-		if (glob_use_alloca (alloca_used, home_len + rest_len + 1))
-		  dirname = alloca_account (home_len + rest_len + 1,
-					    alloca_used);
-		else
+		if (!char_array_set_str (&dirname, p->pw_dir))
 		  {
-		    dirname = malloc (home_len + rest_len + 1);
-		    if (dirname == NULL)
-		      {
-			scratch_buffer_free (&pwtmpbuf);
-			retval = GLOB_NOSPACE;
-			goto out;
-		      }
-		    malloc_dirname = 1;
+		    scratch_buffer_free (&pwtmpbuf);
+		    goto err_nospace;
 		  }
-		d = mempcpy (dirname, p->pw_dir, home_len);
-		if (end_name != NULL)
-		  d = mempcpy (d, end_name, rest_len);
-		*d = '\0';
-
-		dirlen = home_len + rest_len;
-		dirname_modified = 1;
+		dirlen = strlen (p->pw_dir);
+		dirname_modified = true;
 	      }
 	    else
 	      {
@@ -894,37 +849,32 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  goto nospace;
 	pglob->gl_pathv = new_gl_pathv;
 
-	if (flags & GLOB_MARK && is_dir (dirname, flags, pglob))
+	if (flags & GLOB_MARK
+	    && is_dir (char_array_str (&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 = mempcpy (pglob->gl_pathv[newcount],
+			 char_array_str (&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] = strdup (char_array_str (&dirname));
+	    if (pglob->gl_pathv[newcount] == NULL)
+	      goto nospace;
 	  }
 	pglob->gl_pathv[++newcount] = NULL;
 	++pglob->gl_pathc;
 	pglob->gl_flags = flags;
-
-	return 0;
+	goto out;
     }
 
-  meta = __glob_pattern_type (dirname, !(flags & GLOB_NOESCAPE));
+  meta = __glob_pattern_type (char_array_str (&dirname),
+			      !(flags & GLOB_NOESCAPE));
   /* meta is 1 if correct glob pattern containing metacharacters.
      If meta has bit (1 << 2) set, it means there was an unterminated
      [ which we handle the same, using fnmatch.  Broken unterminated
@@ -937,15 +887,15 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	 the pattern in each directory found.  */
       size_t i;
 
-      if (!(flags & GLOB_NOESCAPE) && dirlen > 0 && dirname[dirlen - 1] == '\\')
+      if (!(flags & GLOB_NOESCAPE) && dirlen > 0
+	  && char_array_pos (&dirname, dirlen - 1) == '\\')
 	{
 	  /* "foo\\/bar".  Remove the final backslash from dirname
 	     if it has not been quoted.  */
-	  char *p = (char *) &dirname[dirlen - 1];
-
-	  while (p > dirname && p[-1] == '\\') --p;
-	  if ((&dirname[dirlen] - p) & 1)
-	    *(char *) &dirname[--dirlen] = '\0';
+	  size_t p = dirlen - 1;
+	  while (p > 0 && char_array_pos (&dirname, p - 1) == '\\') --p;
+	  if ((dirlen - p) & 1)
+	    char_array_crop (&dirname, --dirlen);
 	}
 
       if (__glibc_unlikely ((flags & GLOB_ALTDIRFUNC) != 0))
@@ -959,7 +909,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	  dirs.gl_lstat = pglob->gl_lstat;
 	}
 
-      status = __glob (dirname,
+      status = __glob (char_array_str (&dirname),
 		       ((flags & (GLOB_ERR | GLOB_NOESCAPE | GLOB_ALTDIRFUNC))
 			| GLOB_NOSORT | GLOB_ONLYDIR),
 		       errfunc, &dirs);
@@ -1006,8 +956,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      globfree (&dirs);
 	      globfree (pglob);
 	      pglob->gl_pathc = 0;
-	      retval = GLOB_NOSPACE;
-	      goto out;
+	      goto err_nospace;
 	    }
 	}
 
@@ -1029,8 +978,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		{
 		nospace2:
 		  globfree (&dirs);
-		  retval = GLOB_NOSPACE;
-		  goto out;
+		  goto err_nospace;
 		}
 
 	      new_gl_pathv = realloc (pglob->gl_pathv,
@@ -1045,8 +993,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 		  globfree (&dirs);
 		  globfree (pglob);
 		  pglob->gl_pathc = 0;
-		  retval = GLOB_NOSPACE;
-		  goto out;
+		  goto err_nospace;
 		}
 
 	      ++pglob->gl_pathc;
@@ -1072,7 +1019,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 
       if (meta & GLOBPAT_BACKSLASH)
 	{
-	  char *p = strchr (dirname, '\\'), *q;
+	  char *p = strchr (char_array_str (&dirname), '\\'), *q;
 	  /* We need to unescape the dirname string.  It is certainly
 	     allocated by alloca, as otherwise filename would be NULL
 	     or dirname wouldn't contain backslashes.  */
@@ -1089,12 +1036,12 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      ++q;
 	    }
 	  while (*p++ != '\0');
-	  dirname_modified = 1;
+	  dirname_modified = true;
 	}
       if (dirname_modified)
 	flags &= ~(GLOB_NOCHECK | GLOB_NOMAGIC);
-      status = glob_in_dir (filename, dirname, flags, errfunc, pglob,
-			    alloca_used);
+      status = glob_in_dir (filename, char_array_str (&dirname), flags,
+			    errfunc, pglob, alloca_used);
       if (status != 0)
 	{
 	  if (status == GLOB_NOMATCH && flags != orig_flags
@@ -1112,14 +1059,13 @@ __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 (char_array_str (&dirname),
 			    &pglob->gl_pathv[old_pathc + pglob->gl_offs],
 			    pglob->gl_pathc - old_pathc))
 	    {
 	      globfree (pglob);
 	      pglob->gl_pathc = 0;
-	      retval = GLOB_NOSPACE;
-	      goto out;
+	      goto err_nospace;
 	    }
 	}
     }
@@ -1138,8 +1084,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
 	      {
 		globfree (pglob);
 		pglob->gl_pathc = 0;
-		retval = GLOB_NOSPACE;
-		goto out;
+		goto err_nospace;
 	      }
 	    strcpy (&new[len - 2], "/");
 	    pglob->gl_pathv[i] = new;
@@ -1155,10 +1100,12 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
     }
 
  out:
-  if (__glibc_unlikely (malloc_dirname))
-    free (dirname);
-
+  char_array_free (&dirname);
   return retval;
+
+ err_nospace:
+  char_array_free (&dirname);
+  return GLOB_NOSPACE;
 }
 #if defined _LIBC && !defined __glob
 versioned_symbol (libc, __glob, glob, GLIBC_2_27);
-- 
2.7.4

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

* Re: [PATCH v2 12/12] posix: Remove VLA usage for internal fnmatch implementation
  2018-02-05 13:28 ` [PATCH v2 12/12] posix: Remove VLA usage for internal fnmatch implementation Adhemerval Zanella
@ 2018-02-05 14:01   ` Andreas Schwab
  2018-02-05 15:15     ` Adhemerval Zanella
  0 siblings, 1 reply; 20+ messages in thread
From: Andreas Schwab @ 2018-02-05 14:01 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On Feb 05 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c
> index eadb343..831e5ba 100644
> --- a/posix/fnmatch_loop.c
> +++ b/posix/fnmatch_loop.c
> @@ -493,26 +493,20 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
>  			  {
>  			    int32_t table_size;
>  			    const int32_t *symb_table;
> -# if WIDE_CHAR_VERSION
> -			    char str[c1];
> -			    unsigned int strcnt;
> -# else
> -#  define str (startp + 1)
> -# endif
> +			    struct char_array str;
> +			    char_array_init_empty (&str);
>  			    const unsigned char *extra;
>  			    int32_t idx;
>  			    int32_t elem;
>  			    int32_t second;
>  			    int32_t hash;
>  
> -# if WIDE_CHAR_VERSION
>  			    /* We have to convert the name to a single-byte
>  			       string.  This is possible since the names
>  			       consist of ASCII characters and the internal
>  			       representation is UCS4.  */
> -			    for (strcnt = 0; strcnt < c1; ++strcnt)
> -			      str[strcnt] = startp[1 + strcnt];
> -#endif
> +			    for (size_t strcnt = 0; strcnt < c1; ++strcnt)
> +			      char_array_append_char (&str, startp[1 + strcnt]);
>  
>  			    table_size =
>  			      _NL_CURRENT_WORD (LC_COLLATE,

That needs to be removed altogether, see
<http://sourceware.org/ml/libc-alpha/2017-04/msg00068.html>.

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] 20+ messages in thread

* Re: [PATCH v2 09/12] getlogin_r: switch Linux variant to struct scratch_buffer
  2018-02-05 13:27 ` [PATCH v2 09/12] getlogin_r: switch Linux variant to struct scratch_buffer Adhemerval Zanella
@ 2018-02-05 14:10   ` Florian Weimer
  2018-02-05 15:08     ` Adhemerval Zanella
  0 siblings, 1 reply; 20+ messages in thread
From: Florian Weimer @ 2018-02-05 14:10 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On 02/05/2018 02:27 PM, Adhemerval Zanella wrote:
> From: Florian Weimer <fweimer@redhat.com>
> 
> 	[BZ #18023]
> 	* sysdeps/unix/sysv/linux/getlogin_r.c (__getlogin_r_loginuid):
> 	Use scratch_buffer instead of extend_alloca.

Still looks okay to me.  Do you want me to commit it?

Thanks,
Florian

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

* Re: [PATCH v2 09/12] getlogin_r: switch Linux variant to struct scratch_buffer
  2018-02-05 14:10   ` Florian Weimer
@ 2018-02-05 15:08     ` Adhemerval Zanella
  2018-02-05 15:57       ` Florian Weimer
  0 siblings, 1 reply; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 15:08 UTC (permalink / raw)
  To: Florian Weimer; +Cc: libc-alpha



On 05/02/2018 12:05, Florian Weimer wrote:
> On 02/05/2018 02:27 PM, Adhemerval Zanella wrote:
>> From: Florian Weimer <fweimer@redhat.com>
>>
>>     [BZ #18023]
>>     * sysdeps/unix/sysv/linux/getlogin_r.c (__getlogin_r_loginuid):
>>     Use scratch_buffer instead of extend_alloca.
> 
> Still looks okay to me.  Do you want me to commit it?
> 
> Thanks,
> Florian

I can do it for you.

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

* Re: [PATCH v2 12/12] posix: Remove VLA usage for internal fnmatch implementation
  2018-02-05 14:01   ` Andreas Schwab
@ 2018-02-05 15:15     ` Adhemerval Zanella
  2018-02-05 20:23       ` Adhemerval Zanella
  0 siblings, 1 reply; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 15:15 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: libc-alpha



On 05/02/2018 11:40, Andreas Schwab wrote:
> On Feb 05 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> 
>> diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c
>> index eadb343..831e5ba 100644
>> --- a/posix/fnmatch_loop.c
>> +++ b/posix/fnmatch_loop.c
>> @@ -493,26 +493,20 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
>>  			  {
>>  			    int32_t table_size;
>>  			    const int32_t *symb_table;
>> -# if WIDE_CHAR_VERSION
>> -			    char str[c1];
>> -			    unsigned int strcnt;
>> -# else
>> -#  define str (startp + 1)
>> -# endif
>> +			    struct char_array str;
>> +			    char_array_init_empty (&str);
>>  			    const unsigned char *extra;
>>  			    int32_t idx;
>>  			    int32_t elem;
>>  			    int32_t second;
>>  			    int32_t hash;
>>  
>> -# if WIDE_CHAR_VERSION
>>  			    /* We have to convert the name to a single-byte
>>  			       string.  This is possible since the names
>>  			       consist of ASCII characters and the internal
>>  			       representation is UCS4.  */
>> -			    for (strcnt = 0; strcnt < c1; ++strcnt)
>> -			      str[strcnt] = startp[1 + strcnt];
>> -#endif
>> +			    for (size_t strcnt = 0; strcnt < c1; ++strcnt)
>> +			      char_array_append_char (&str, startp[1 + strcnt]);
>>  
>>  			    table_size =
>>  			      _NL_CURRENT_WORD (LC_COLLATE,
> 
> That needs to be removed altogether, see
> <http://sourceware.org/ml/libc-alpha/2017-04/msg00068.html>.

Nice, I missed your patch.  It seems to apply clean, I will review on the original
thread.

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

* Re: [PATCH v2 09/12] getlogin_r: switch Linux variant to struct scratch_buffer
  2018-02-05 15:08     ` Adhemerval Zanella
@ 2018-02-05 15:57       ` Florian Weimer
  0 siblings, 0 replies; 20+ messages in thread
From: Florian Weimer @ 2018-02-05 15:57 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On 02/05/2018 04:06 PM, Adhemerval Zanella wrote:
> 
> 
> On 05/02/2018 12:05, Florian Weimer wrote:
>> On 02/05/2018 02:27 PM, Adhemerval Zanella wrote:
>>> From: Florian Weimer <fweimer@redhat.com>
>>>
>>>      [BZ #18023]
>>>      * sysdeps/unix/sysv/linux/getlogin_r.c (__getlogin_r_loginuid):
>>>      Use scratch_buffer instead of extend_alloca.
>>
>> Still looks okay to me.  Do you want me to commit it?
>>
>> Thanks,
>> Florian
> 
> I can do it for you.

Okay, please do.

Thanks,
Florian

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

* Re: [PATCH v2 12/12] posix: Remove VLA usage for internal fnmatch implementation
  2018-02-05 15:15     ` Adhemerval Zanella
@ 2018-02-05 20:23       ` Adhemerval Zanella
  2018-02-06 13:50         ` Andreas Schwab
  0 siblings, 1 reply; 20+ messages in thread
From: Adhemerval Zanella @ 2018-02-05 20:23 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: libc-alpha



On 05/02/2018 13:08, Adhemerval Zanella wrote:
> 
> 
> On 05/02/2018 11:40, Andreas Schwab wrote:
>> On Feb 05 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
>>
>>> diff --git a/posix/fnmatch_loop.c b/posix/fnmatch_loop.c
>>> index eadb343..831e5ba 100644
>>> --- a/posix/fnmatch_loop.c
>>> +++ b/posix/fnmatch_loop.c
>>> @@ -493,26 +493,20 @@ FCT (const CHAR *pattern, const CHAR *string, const CHAR *string_end,
>>>  			  {
>>>  			    int32_t table_size;
>>>  			    const int32_t *symb_table;
>>> -# if WIDE_CHAR_VERSION
>>> -			    char str[c1];
>>> -			    unsigned int strcnt;
>>> -# else
>>> -#  define str (startp + 1)
>>> -# endif
>>> +			    struct char_array str;
>>> +			    char_array_init_empty (&str);
>>>  			    const unsigned char *extra;
>>>  			    int32_t idx;
>>>  			    int32_t elem;
>>>  			    int32_t second;
>>>  			    int32_t hash;
>>>  
>>> -# if WIDE_CHAR_VERSION
>>>  			    /* We have to convert the name to a single-byte
>>>  			       string.  This is possible since the names
>>>  			       consist of ASCII characters and the internal
>>>  			       representation is UCS4.  */
>>> -			    for (strcnt = 0; strcnt < c1; ++strcnt)
>>> -			      str[strcnt] = startp[1 + strcnt];
>>> -#endif
>>> +			    for (size_t strcnt = 0; strcnt < c1; ++strcnt)
>>> +			      char_array_append_char (&str, startp[1 + strcnt]);
>>>  
>>>  			    table_size =
>>>  			      _NL_CURRENT_WORD (LC_COLLATE,
>>
>> That needs to be removed altogether, see
>> <http://sourceware.org/ml/libc-alpha/2017-04/msg00068.html>.
> 
> Nice, I missed your patch.  It seems to apply clean, I will review on the original
> thread.

Re-checking the bug reports related to the issues your patch aims to fix, I
noted on BZ#14185 comment #5 Jeff Law stated SuSE used to pack an out of tree
fix but non-longer do it. Do you know why exactly?

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

* Re: [PATCH v2 12/12] posix: Remove VLA usage for internal fnmatch implementation
  2018-02-05 20:23       ` Adhemerval Zanella
@ 2018-02-06 13:50         ` Andreas Schwab
  0 siblings, 0 replies; 20+ messages in thread
From: Andreas Schwab @ 2018-02-06 13:50 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

On Feb 05 2018, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:

> Re-checking the bug reports related to the issues your patch aims to fix, I
> noted on BZ#14185 comment #5 Jeff Law stated SuSE used to pack an out of tree
> fix but non-longer do it. Do you know why exactly?

I can't answer that question, I wasn't around at that time (the
changelog talks about "obsolete patch", which I don't quite follow).
But this bug is about matching the string, not processing the pattern,
so it is an unrelated issue.

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] 20+ messages in thread

end of thread, other threads:[~2018-02-06  8:58 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-05 13:27 [PATCH v2 00/12] posix: glob/fnmatch fixes and refactor Adhemerval Zanella
2018-02-05 13:27 ` [PATCH v2 09/12] getlogin_r: switch Linux variant to struct scratch_buffer Adhemerval Zanella
2018-02-05 14:10   ` Florian Weimer
2018-02-05 15:08     ` Adhemerval Zanella
2018-02-05 15:57       ` Florian Weimer
2018-02-05 13:27 ` [PATCH v2 08/12] posix: Remove all alloca usage in glob Adhemerval Zanella
2018-02-05 13:27 ` [PATCH v2 03/12] posix: Remove alloca usage for GLOB_BRACE on glob Adhemerval Zanella
2018-02-05 13:27 ` [PATCH v2 06/12] posix: Remove alloca usage on glob user_name Adhemerval Zanella
2018-02-05 13:28 ` [PATCH v2 07/12] posix: Use char_array for home_dir in glob Adhemerval Zanella
2018-02-05 13:28 ` [PATCH v2 01/12] malloc: Add specialized dynarray for C strings Adhemerval Zanella
2018-02-05 13:28 ` [PATCH v2 05/12] posix: Use dynarray for globname in glob Adhemerval Zanella
2018-02-05 13:28 ` [PATCH v2 11/12] posix: Remove alloca usage for internal fnmatch implementation Adhemerval Zanella
2018-02-05 13:28 ` [PATCH v2 04/12] posix: Remove alloca usage on glob dirname Adhemerval Zanella
2018-02-05 13:28 ` [PATCH v2 12/12] posix: Remove VLA usage for internal fnmatch implementation Adhemerval Zanella
2018-02-05 14:01   ` Andreas Schwab
2018-02-05 15:15     ` Adhemerval Zanella
2018-02-05 20:23       ` Adhemerval Zanella
2018-02-06 13:50         ` Andreas Schwab
2018-02-05 13:28 ` [PATCH v2 10/12] posix: Replace alloca usage with scratch_buffer for fnmatch Adhemerval Zanella
2018-02-05 13:40 ` [PATCH v2 02/12] posix: Use char_array for internal glob dirname 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).