* [PATCH 03/17] support: Add optstring support
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
2017-06-08 21:13 ` [PATCH 02/17] posix: Allow glob to match dangling symlinks [BZ #866] Adhemerval Zanella
@ 2017-06-08 21:13 ` Adhemerval Zanella
2017-06-13 9:22 ` Florian Weimer
2017-06-08 21:14 ` [PATCH 09/17] posix: Remove glob GET_LOGIN_NAME_MAX usage Adhemerval Zanella
` (14 subsequent siblings)
16 siblings, 1 reply; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:13 UTC (permalink / raw)
To: libc-alpha
This patch adds an option to test to add small command line option
through CMDLINE_OPTSTRING define. For instance:
#define CMDLINE_OPTSTRING "vd"
static void
cmdline_process_function (int c)
{
switch (c):
'v':
/* process '-v' option. */
break;
'd':
/* process '-d' option. */
break;
}
#define CMDLINE_PROCESS cmdline_process_function
It will add both '-v' and '-d' along with already default long options.
* support/support_test_main.c (support_test_main): Use optstring
member for option string in getopt_long.
* support/test-driver.c: Add comment about CMDLINE_OPTSTRING.
(CMDLINE_OPTSTRING): New define.
* support/test-driver.h (test_config): Add optstring member.
---
support/support_test_main.c | 3 ++-
support/test-driver.c | 9 +++++++++
support/test-driver.h | 1 +
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/support/support_test_main.c b/support/support_test_main.c
index 914d64f..3c411a4 100644
--- a/support/support_test_main.c
+++ b/support/support_test_main.c
@@ -211,7 +211,8 @@ support_test_main (int argc, char **argv, const struct test_config *config)
mallopt (M_PERTURB, 42);
}
- while ((opt = getopt_long (argc, argv, "+", options, NULL)) != -1)
+ while ((opt = getopt_long (argc, argv, config->optstring, options, NULL))
+ != -1)
switch (opt)
{
case '?':
diff --git a/support/test-driver.c b/support/test-driver.c
index 482066d..f5a2388 100644
--- a/support/test-driver.c
+++ b/support/test-driver.c
@@ -93,6 +93,10 @@
has this type:
void CMDLINE_PROCESS (int);
+
+ If the program also to process custom default short command line
+ argument (similar to getopt) it must define CMDLINE_OPTSTRING
+ with the expected options (for instance "vb").
*/
#include <support/test-driver.h>
@@ -151,6 +155,11 @@ main (int argc, char **argv)
#ifdef CMDLINE_PROCESS
test_config.cmdline_function = CMDLINE_PROCESS;
#endif
+#ifdef CMDLINE_OPTSTRING
+ test_config.optstring = CMDLINE_OPTSTRING;
+#else
+ test_config.optstring = "+";
+#endif
return support_test_main (argc, argv, &test_config);
}
diff --git a/support/test-driver.h b/support/test-driver.h
index af1971a..a8fe9c3 100644
--- a/support/test-driver.h
+++ b/support/test-driver.h
@@ -35,6 +35,7 @@ struct test_config
int expected_status; /* Expected exit status. */
int expected_signal; /* If non-zero, expect termination by signal. */
char no_mallopt; /* Boolean flag to disable mallopt. */
+ const char *optstring; /* Short command line options. */
};
enum
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 02/17] posix: Allow glob to match dangling symlinks [BZ #866]
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
@ 2017-06-08 21:13 ` Adhemerval Zanella
2017-06-08 21:13 ` [PATCH 03/17] support: Add optstring support Adhemerval Zanella
` (15 subsequent siblings)
16 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:13 UTC (permalink / raw)
To: libc-alpha
This patch makes glob match dangling symlinks. Compared to other glob
implementation (*BSD, bash, musl, and other shells as well), GLIBC seems
the be the only one that does not match dangling symlinks. As for
comment #5 in BZ #866, POSIX does not have any strict specification for
dangling symlinks match and it is reasonable that trying to glob everything
in a path should return all types of files (such as for a 'rm *'). Also,
comment #7 shows even more example where GLIBC current behavior is
unexepected.
I avoided adding another GNU specific flag to set this behavior and
instead make it the default. Although this change the semanthic from
previous implementation, I think adding another compat symbol to be
really unecessary as from aforementioned reasons (current behavior not
defined in any standard, general idea of different implementation is
to list dangling symbols).
Checked on x86_64-linux-gnu.
* posix/Makefile (tests): Add tst-glob_symlinks and remove tst-glob3.
* posix/bug-glob1.c: Remove file.
* posix/glob.c (glob): Match dangling symlinks.
(link_exists2_p): Remove function.
(link_exists_p): Likewise.
* posix/tst-glob_symlinks.c: New file.
* sysdeps/gnu/glob64.c (__stat): Redefine to __lstat.
* sysdeps/unix/sysv/linux/i386/glob64.c (__stat): Likewise.
---
posix/Makefile | 5 +-
posix/bug-glob1.c | 88 ----------------------
posix/glob.c | 122 ++++++++-----------------------
posix/tst-glob_symlinks.c | 133 ++++++++++++++++++++++++++++++++++
sysdeps/gnu/glob64.c | 4 +-
sysdeps/unix/sysv/linux/i386/glob64.c | 4 +-
6 files changed, 171 insertions(+), 185 deletions(-)
delete mode 100644 posix/bug-glob1.c
create mode 100644 posix/tst-glob_symlinks.c
diff --git a/posix/Makefile b/posix/Makefile
index 52b022c..2c0c9f9 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -69,7 +69,7 @@ tests := test-errno tstgetopt testfnm runtests runptests \
tst-mmap tst-mmap-offset tst-getaddrinfo tst-truncate \
tst-truncate64 tst-fork tst-fnmatch tst-regexloc tst-dir \
tst-chmod bug-regex1 bug-regex2 bug-regex3 bug-regex4 \
- tst-gnuglob tst-regex bug-regex6 bug-regex7 \
+ tst-gnuglob tst-glob_symlinks tst-regex bug-regex6 bug-regex7 \
bug-regex8 bug-regex9 bug-regex10 bug-regex11 bug-regex12 \
bug-regex13 bug-regex14 bug-regex15 bug-regex16 \
bug-regex17 bug-regex18 bug-regex19 \
@@ -79,7 +79,7 @@ tests := test-errno tstgetopt testfnm runtests runptests \
tst-nice tst-nanosleep tst-regex2 \
transbug tst-rxspencer tst-pcre tst-boost \
bug-ga1 tst-vfork1 tst-vfork2 tst-vfork3 tst-waitid \
- tst-getaddrinfo2 bug-glob1 bug-glob2 bug-glob3 tst-sysconf \
+ tst-getaddrinfo2 bug-glob2 bug-glob3 tst-sysconf \
tst-execvp1 tst-execvp2 tst-execlp1 tst-execlp2 \
tst-execv1 tst-execv2 tst-execl1 tst-execl2 \
tst-execve1 tst-execve2 tst-execle1 tst-execle2 \
@@ -249,7 +249,6 @@ tst-rxspencer-ARGS = --utf8 rxspencer/tests
tst-rxspencer-no-utf8-ARGS = rxspencer/tests
tst-pcre-ARGS = PCRE.tests
tst-boost-ARGS = BOOST.tests
-bug-glob1-ARGS = "$(objpfx)"
tst-execvp3-ARGS = --test-dir=$(objpfx)
testcases.h: TESTS TESTS2C.sed
diff --git a/posix/bug-glob1.c b/posix/bug-glob1.c
deleted file mode 100644
index 05c2da7..0000000
--- a/posix/bug-glob1.c
+++ /dev/null
@@ -1,88 +0,0 @@
-/* Test case for globbing dangling symlink. By Ulrich Drepper. */
-#include <errno.h>
-#include <error.h>
-#include <glob.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-
-
-static void prepare (int argc, char *argv[]);
-#define PREPARE prepare
-static int do_test (void);
-#define TEST_FUNCTION do_test ()
-
-#include "../test-skeleton.c"
-
-
-static char *fname;
-
-static void
-prepare (int argc, char *argv[])
-{
- if (argc < 2)
- error (EXIT_FAILURE, 0, "missing argument");
-
- size_t len = strlen (argv[1]);
- static const char ext[] = "globXXXXXX";
- fname = malloc (len + sizeof (ext));
- if (fname == NULL)
- error (EXIT_FAILURE, errno, "cannot create temp file");
- again:
- strcpy (stpcpy (fname, argv[1]), ext);
- fname = mktemp (fname);
- if (fname == NULL || *fname == '\0')
- error (EXIT_FAILURE, errno, "cannot create temp file name");
- if (symlink ("bug-glob1-does-not-exist", fname) != 0)
- {
- if (errno == EEXIST)
- goto again;
-
- error (EXIT_FAILURE, errno, "cannot create symlink");
- }
- add_temp_file (fname);
-}
-
-
-static int
-do_test (void)
-{
- glob_t gl;
- int retval = 0;
- int e;
-
- e = glob (fname, 0, NULL, &gl);
- if (e == 0)
- {
- printf ("glob(\"%s\") succeeded\n", fname);
- retval = 1;
- }
- globfree (&gl);
-
- size_t fnamelen = strlen (fname);
- char buf[fnamelen + 2];
-
- strcpy (buf, fname);
- buf[fnamelen - 1] = '?';
- e = glob (buf, 0, NULL, &gl);
- if (e == 0)
- {
- printf ("glob(\"%s\") succeeded\n", buf);
- retval = 1;
- }
- globfree (&gl);
-
- strcpy (buf, fname);
- buf[fnamelen] = '*';
- buf[fnamelen + 1] = '\0';
- e = glob (buf, 0, NULL, &gl);
- if (e == 0)
- {
- printf ("glob(\"%s\") succeeded\n", buf);
- retval = 1;
- }
- globfree (&gl);
-
- return retval;
-}
diff --git a/posix/glob.c b/posix/glob.c
index 3c6b033..250bff1 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -71,8 +71,8 @@
# define readdir(str) __readdir64 (str)
# define getpwnam_r(name, bufp, buf, len, res) \
__getpwnam_r (name, bufp, buf, len, res)
-# ifndef __stat64
-# define __stat64(fname, buf) __xstat64 (_STAT_VER, fname, buf)
+# ifndef __lstat64
+# define __lstat64(fname, buf) __lxstat64 (_STAT_VER, fname, buf)
# endif
# define struct_stat64 struct stat64
#else /* !_LIBC */
@@ -1049,9 +1049,9 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
/* Return the directory if we don't check for error or if it exists. */
if ((flags & GLOB_NOCHECK)
|| (((__builtin_expect (flags & GLOB_ALTDIRFUNC, 0))
- ? ((*pglob->gl_stat) (dirname, &st) == 0
+ ? ((*pglob->gl_lstat) (dirname, &st) == 0
&& S_ISDIR (st.st_mode))
- : (__stat64 (dirname, &st64) == 0 && S_ISDIR (st64.st_mode)))))
+ : (__lstat64 (dirname, &st64) == 0 && S_ISDIR (st64.st_mode)))))
{
size_t newcount = pglob->gl_pathc + pglob->gl_offs;
char **new_gl_pathv;
@@ -1318,10 +1318,10 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
for (i = oldcount; i < pglob->gl_pathc + pglob->gl_offs; ++i)
if ((__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
- ? ((*pglob->gl_stat) (pglob->gl_pathv[i], &st) == 0
- && S_ISDIR (st.st_mode))
- : (__stat64 (pglob->gl_pathv[i], &st64) == 0
- && S_ISDIR (st64.st_mode))))
+ ? ((*pglob->gl_lstat) (pglob->gl_pathv[i], &st) == 0
+ && (S_ISDIR (st.st_mode) || S_ISLNK (st.st_mode)))
+ : (__lstat64 (pglob->gl_pathv[i], &st64) == 0
+ && (S_ISDIR (st64.st_mode) || S_ISLNK (st64.st_mode)))))
{
size_t len = strlen (pglob->gl_pathv[i]) + 2;
char *new = realloc (pglob->gl_pathv[i], len);
@@ -1500,54 +1500,6 @@ weak_alias (__glob_pattern_p, glob_pattern_p)
# endif
#endif
-
-/* We put this in a separate function mainly to allow the memory
- allocated with alloca to be recycled. */
-static int
-__attribute_noinline__
-link_exists2_p (const char *dir, size_t dirlen, const char *fname,
- glob_t *pglob
-# if !defined _LIBC && !HAVE_FSTATAT
- , int flags
-# endif
- )
-{
- size_t fnamelen = strlen (fname);
- char *fullname = __alloca (dirlen + 1 + fnamelen + 1);
- struct stat st;
-
- mempcpy (mempcpy (mempcpy (fullname, dir, dirlen), "/", 1),
- fname, fnamelen + 1);
-
-# if !defined _LIBC && !HAVE_FSTATAT
- if (__builtin_expect ((flags & GLOB_ALTDIRFUNC) == 0, 1))
- {
- struct_stat64 st64;
- return __stat64 (fullname, &st64) == 0;
- }
-# endif
- return (*pglob->gl_stat) (fullname, &st) == 0;
-}
-
-/* Return true if DIR/FNAME exists. */
-static int
-link_exists_p (int dfd, const char *dir, size_t dirlen, const char *fname,
- glob_t *pglob, int flags)
-{
-# if defined _LIBC || HAVE_FSTATAT
- if (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0))
- return link_exists2_p (dir, dirlen, fname, pglob);
- else
- {
- /* dfd cannot be -1 here, because dirfd never returns -1 on
- glibc, or on hosts that have fstatat. */
- struct_stat64 st64;
- return __fxstatat64 (_STAT_VER, dfd, fname, &st64, 0) == 0;
- }
-# else
- return link_exists2_p (dir, dirlen, fname, pglob, flags);
-# endif
-}
#endif /* !defined GLOB_ONLY_P */
@@ -1619,8 +1571,8 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
"/", 1),
pattern, patlen + 1);
if ((__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
- ? (*pglob->gl_stat) (fullname, &ust.st)
- : __stat64 (fullname, &ust.st64)) == 0)
+ ? (*pglob->gl_lstat) (fullname, &ust.st)
+ : __lstat64 (fullname, &ust.st64)) == 0)
/* We found this file to be existing. Now tell the rest
of the function to copy this name into the result. */
flags |= GLOB_NOCHECK;
@@ -1642,8 +1594,6 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
}
else
{
- int dfd = (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
- ? -1 : dirfd ((DIR *) stream));
int fnm_flags = ((!(flags & GLOB_PERIOD) ? FNM_PERIOD : 0)
| ((flags & GLOB_NOESCAPE) ? FNM_NOESCAPE : 0)
#if defined _AMIGA || defined VMS
@@ -1679,38 +1629,30 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
if (fnmatch (pattern, d.name, fnm_flags) == 0)
{
- /* If the file we found is a symlink we have to
- make sure the target file exists. */
- if (!readdir_result_might_be_symlink (d)
- || link_exists_p (dfd, directory, dirlen, d.name,
- pglob, flags))
+ if (cur == names->count)
{
- if (cur == names->count)
- {
- struct globnames *newnames;
- size_t count = names->count * 2;
- size_t size = (sizeof (struct globnames)
- + ((count - INITIAL_COUNT)
- * sizeof (char *)));
- 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)
+ struct globnames *newnames;
+ size_t count = names->count * 2;
+ size_t size = (sizeof (struct globnames)
+ + ((count - INITIAL_COUNT)
+ * sizeof (char *)));
+ if (glob_use_alloca (alloca_used, size))
+ newnames = names_alloca
+ = alloca_account (size, alloca_used);
+ else if ((newnames = malloc (size)) == NULL)
goto memory_error;
- }
+ newnames->count = count;
+ newnames->next = names;
+ names = newnames;
+ cur = 0;
+ }
+ names->name[cur] = strdup (d.name);
+ if (names->name[cur] == NULL)
+ goto memory_error;
+ ++cur;
+ ++nfound;
+ if (SIZE_MAX - pglob->gl_offs <= nfound)
+ goto memory_error;
}
}
}
diff --git a/posix/tst-glob_symlinks.c b/posix/tst-glob_symlinks.c
new file mode 100644
index 0000000..c0f1802
--- /dev/null
+++ b/posix/tst-glob_symlinks.c
@@ -0,0 +1,133 @@
+/* Test glob symlinks return (BZ #866).
+ for the filesystem access functions.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <unistd.h>
+#include <limits.h>
+#include <stddef.h>
+#include <glob.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+#include <support/check.h>
+#include <support/temp_file.h>
+
+static void do_prepare (int argc, char *argv[]);
+#define PREPARE do_prepare
+static int do_test (void);
+#include <support/test-driver.c>
+
+static void
+create_link (const char *base, const char *fname, char *linkname,
+ size_t linknamesize)
+{
+ int ntries = 0;
+ while (1)
+ {
+ snprintf (linkname, linknamesize, "%s/%s%02d", test_dir, base,
+ ntries);
+ if (symlink (fname, linkname) == 0)
+ break;
+ if (errno != EEXIST)
+ FAIL_EXIT1 ("symlink failed: %m");
+ if (ntries++ == 10)
+ FAIL_EXIT1 ("symlink failed with EEXIST too many times");
+ }
+ add_temp_file (linkname);
+}
+
+static char valid_link[PATH_MAX];
+static char dangling_link[PATH_MAX];
+static char dangling_dir[PATH_MAX];
+
+static void
+do_prepare (int argc, char *argv[])
+{
+ char *fname;
+
+ create_temp_file ("tst-glob_symlinks.", &fname);
+
+ /* Create a existing symlink. */
+ create_link ("valid-symlink-tst-glob_symlinks", fname, valid_link,
+ sizeof valid_link);
+
+ /* Create a dangling symlink to a file. */
+ int fd = create_temp_file ("dangling-tst-glob_file", &fname);
+ TEST_VERIFY_EXIT (close (fd) == 0);
+ /* It throws an warning at process end due 'add_temp_file' trying to
+ unlink it again. */
+ TEST_VERIFY_EXIT (unlink (fname) == 0);
+ create_link ("dangling-symlink-file-tst-glob", fname, dangling_link,
+ sizeof dangling_link);
+
+ /* Create a dangling symlink to a directory. */
+ char tmpdir[PATH_MAX];
+ snprintf (tmpdir, sizeof tmpdir, "%s/dangling-tst-glob_folder.XXXXXX",
+ test_dir);
+ TEST_VERIFY_EXIT (mkdtemp (tmpdir) != NULL);
+ create_link ("dangling-symlink-dir-tst-glob", tmpdir, dangling_dir,
+ sizeof dangling_dir);
+ TEST_VERIFY_EXIT (rmdir (tmpdir) == 0);
+}
+
+static int
+do_test (void)
+{
+ char buf[PATH_MAX];
+ glob_t gl;
+
+ TEST_VERIFY_EXIT (glob (valid_link, 0, NULL, &gl) == 0);
+ TEST_VERIFY_EXIT (gl.gl_pathc == 1);
+ TEST_VERIFY_EXIT (strcmp (gl.gl_pathv[0], valid_link) == 0);
+ globfree (&gl);
+
+ TEST_VERIFY_EXIT (glob (dangling_link, 0, NULL, &gl) == 0);
+ TEST_VERIFY_EXIT (gl.gl_pathc == 1);
+ TEST_VERIFY_EXIT (strcmp (gl.gl_pathv[0], dangling_link) == 0);
+ globfree (&gl);
+
+ TEST_VERIFY_EXIT (glob (dangling_dir, 0, NULL, &gl) == 0);
+ TEST_VERIFY_EXIT (gl.gl_pathc == 1);
+ TEST_VERIFY_EXIT (strcmp (gl.gl_pathv[0], dangling_dir) == 0);
+ globfree (&gl);
+
+ snprintf (buf, sizeof buf, "%s", dangling_link);
+ buf[strlen(buf) - 1] = '?';
+ TEST_VERIFY_EXIT (glob (buf, 0, NULL, &gl) == 0);
+ TEST_VERIFY_EXIT (gl.gl_pathc == 1);
+ TEST_VERIFY_EXIT (strcmp (gl.gl_pathv[0], dangling_link) == 0);
+ globfree (&gl);
+
+ /* glob should handle dangling symbol as normal file, so <file>? should
+ return an empty string. */
+ snprintf (buf, sizeof buf, "%s?", dangling_link);
+ TEST_VERIFY_EXIT (glob (buf, 0, NULL, &gl) != 0);
+ globfree (&gl);
+
+ snprintf (buf, sizeof buf, "%s*", dangling_link);
+ TEST_VERIFY_EXIT (glob (buf, 0, NULL, &gl) == 0);
+ TEST_VERIFY_EXIT (gl.gl_pathc == 1);
+ TEST_VERIFY_EXIT (strcmp (gl.gl_pathv[0], dangling_link) == 0);
+ globfree (&gl);
+
+ return 0;
+}
diff --git a/sysdeps/gnu/glob64.c b/sysdeps/gnu/glob64.c
index d1e4e6f..250ff07 100644
--- a/sysdeps/gnu/glob64.c
+++ b/sysdeps/gnu/glob64.c
@@ -12,8 +12,8 @@
#undef stat
#define stat stat64
-#undef __stat
-#define __stat(file, buf) __xstat64 (_STAT_VER, file, buf)
+#undef __lstat
+#define __lstat(file, buf) __lxstat64 (_STAT_VER, file, buf)
#define NO_GLOB_PATTERN_P 1
diff --git a/sysdeps/unix/sysv/linux/i386/glob64.c b/sysdeps/unix/sysv/linux/i386/glob64.c
index 9c7abd8..2dcbe33 100644
--- a/sysdeps/unix/sysv/linux/i386/glob64.c
+++ b/sysdeps/unix/sysv/linux/i386/glob64.c
@@ -30,8 +30,8 @@
#undef stat
#define stat stat64
-#undef __stat
-#define __stat(file, buf) __xstat64 (_STAT_VER, file, buf)
+#undef __lstat
+#define __lstat(file, buf) __lxstat64 (_STAT_VER, file, buf)
#define NO_GLOB_PATTERN_P 1
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 00/17] posix: glob fixes and refactor
@ 2017-06-08 21:13 Adhemerval Zanella
2017-06-08 21:13 ` [PATCH 02/17] posix: Allow glob to match dangling symlinks [BZ #866] Adhemerval Zanella
` (16 more replies)
0 siblings, 17 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:13 UTC (permalink / raw)
To: libc-alpha
The set fixes some long standing glob issues (gnulib sync and dandling
symlinks) the refactor current, adjust its tests for libsupport and
refactor current implementation to use a dynarray implementation for
C strings.
To correct provide a C string analogous using dynarray I had to access
its private member directly, so I created a specialized implementation
(malloc/char_array.c) for it. It currently works similar to dynarray,
by including its header which defines static functions. If it is the
case, one could its function as an internal API. I added some
function only to manipulate mainly for the work to refactor glob.
The idea to refactor is mainly to remove the alloca and internal buffer
handling boilerplate usage. So it does not trace its possible stack
usage using 'alloca_used' variable, which is somewhat fragile (since
it just track glob usage itself). I used 256 bytes for default
string size (for initial stack allocation), but we can also tune it
if required.
I did not and will not take care of the non-unix code on glob.c (it
is probably broken after the glob refactor). In fact I would like
to cleanup this implementation even further to remove all code not
really used for GLIBC if gnulib developers want to not keep the
implementation at sync.
Adhemerval Zanella (16):
posix: Sync glob with gnulib [BZ #1062]
posix: Allow glob to match dangling symlinks [BZ #866]
support: Add optstring support
posix: Adjust glob tests to libsupport
posix: Consolidate glob implementation
malloc: Add specialized dynarray for C strings
posix: Use char_array for internal glob dirname
posix: Remove glob GET_LOGIN_NAME_MAX usage
posix: User LOGIN_NAME_MAX for all user name in glob
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 all alloca usage in glob
posix: Use char_array for home_dir in glob
posix: Add common function to get home directory
posix: More check for overflow allocation in glob
Florian Weimer (1):
posix: Rewrite to use struct scratch_buffer instead of extend_alloca
malloc/Makefile | 1 +
malloc/char_array.c | 256 +++++
malloc/malloc-internal.h | 14 +
malloc/tst-char_array.c | 107 ++
posix/Makefile | 17 +-
posix/bug-glob1.c | 88 --
posix/bug-glob3.c | 45 -
posix/glob.c | 1166 ++++++++------------
posix/glob_internal.h | 57 +
posix/glob_pattern_p.c | 29 +
posix/globfree.c | 37 +
posix/globfree64.c | 27 +
posix/globtest.c | 166 +--
posix/tst-glob_basic.c | 41 +
posix/tst-glob_common.c | 103 ++
posix/{bug-glob2.c => tst-glob_memory.c} | 100 +-
posix/tst-glob_symlinks.c | 133 +++
posix/tst-gnuglob.c | 109 +-
support/support_test_main.c | 3 +-
support/test-driver.c | 9 +
support/test-driver.h | 1 +
sysdeps/gnu/glob64.c | 25 -
sysdeps/unix/sysv/linux/Makefile | 2 +-
sysdeps/unix/sysv/linux/alpha/Makefile | 4 -
sysdeps/unix/sysv/linux/alpha/{glob.c => glob64.c} | 11 -
sysdeps/unix/sysv/linux/alpha/globfree.c | 36 +
sysdeps/unix/sysv/linux/arm/glob64.c | 1 -
sysdeps/unix/sysv/linux/glob.c | 23 +
sysdeps/unix/sysv/linux/glob64.c | 55 +
sysdeps/unix/sysv/linux/globfree.c | 23 +
sysdeps/unix/sysv/linux/globfree64.c | 0
sysdeps/unix/sysv/linux/i386/alphasort64.c | 2 +-
sysdeps/unix/sysv/linux/i386/getdents64.c | 2 +-
sysdeps/unix/sysv/linux/i386/readdir64.c | 2 +-
sysdeps/unix/sysv/linux/i386/readdir64_r.c | 2 +-
sysdeps/unix/sysv/linux/i386/versionsort64.c | 2 +-
sysdeps/unix/sysv/linux/m68k/glob64.c | 1 -
sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c | 1 -
sysdeps/unix/sysv/linux/{i386 => }/olddirent.h | 0
.../unix/sysv/linux/{i386/glob64.c => oldglob.c} | 51 +-
sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c | 1 -
sysdeps/unix/sysv/linux/s390/s390-32/glob64.c | 20 +
sysdeps/unix/sysv/linux/s390/s390-32/oldglob.c | 2 +
sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c | 1 -
sysdeps/unix/sysv/linux/wordsize-64/glob64.c | 2 -
sysdeps/unix/sysv/linux/x86_64/x32/glob.c | 1 -
sysdeps/wordsize-64/glob.c | 8 -
sysdeps/wordsize-64/glob64.c | 1 -
48 files changed, 1558 insertions(+), 1230 deletions(-)
create mode 100644 malloc/char_array.c
create mode 100644 malloc/tst-char_array.c
delete mode 100644 posix/bug-glob1.c
delete mode 100644 posix/bug-glob3.c
create mode 100644 posix/glob_internal.h
create mode 100644 posix/glob_pattern_p.c
create mode 100644 posix/globfree.c
create mode 100644 posix/globfree64.c
create mode 100644 posix/tst-glob_basic.c
create mode 100644 posix/tst-glob_common.c
rename posix/{bug-glob2.c => tst-glob_memory.c} (76%)
create mode 100644 posix/tst-glob_symlinks.c
delete mode 100644 sysdeps/gnu/glob64.c
rename sysdeps/unix/sysv/linux/alpha/{glob.c => glob64.c} (78%)
create mode 100644 sysdeps/unix/sysv/linux/alpha/globfree.c
delete mode 100644 sysdeps/unix/sysv/linux/arm/glob64.c
create mode 100644 sysdeps/unix/sysv/linux/glob.c
create mode 100644 sysdeps/unix/sysv/linux/glob64.c
create mode 100644 sysdeps/unix/sysv/linux/globfree.c
create mode 100644 sysdeps/unix/sysv/linux/globfree64.c
delete mode 100644 sysdeps/unix/sysv/linux/m68k/glob64.c
delete mode 100644 sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c
rename sysdeps/unix/sysv/linux/{i386 => }/olddirent.h (100%)
rename sysdeps/unix/sysv/linux/{i386/glob64.c => oldglob.c} (62%)
delete mode 100644 sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c
create mode 100644 sysdeps/unix/sysv/linux/s390/s390-32/glob64.c
create mode 100644 sysdeps/unix/sysv/linux/s390/s390-32/oldglob.c
delete mode 100644 sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c
delete mode 100644 sysdeps/unix/sysv/linux/wordsize-64/glob64.c
delete mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/glob.c
delete mode 100644 sysdeps/wordsize-64/glob.c
delete mode 100644 sysdeps/wordsize-64/glob64.c
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 06/17] posix: Rewrite to use struct scratch_buffer instead of extend_alloca
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
` (5 preceding siblings ...)
2017-06-08 21:14 ` [PATCH 07/17] malloc: Add specialized dynarray for C strings Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
2017-06-14 14:08 ` Florian Weimer
2017-06-08 21:14 ` [PATCH 16/17] posix: Add common function to get home directory Adhemerval Zanella
` (9 subsequent siblings)
16 siblings, 1 reply; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 UTC (permalink / raw)
To: libc-alpha; +Cc: Florian Weimer
From: Florian Weimer <fweimer@redhat.com>
This patch removes a lot of boilerplate code to manager buffers for
getpwnam_r.
Checked on x86_64-linux-gnu.
[BZ #18023]
* posix/glob.c (glob): Use struct scratch_buffer instead of
extend_alloca.
---
posix/glob.c | 147 +++++++++++------------------------------------------------
1 file changed, 26 insertions(+), 121 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index dc13e26..425d81b 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -88,13 +88,9 @@
#include <fnmatch.h>
+#include <scratch_buffer.h>
#include "glob_internal.h"
-#ifdef _SC_GETPW_R_SIZE_MAX
-# define GETPW_R_SIZE_MAX() sysconf (_SC_GETPW_R_SIZE_MAX)
-#else
-# define GETPW_R_SIZE_MAX() (-1)
-#endif
#ifdef _SC_LOGIN_NAME_MAX
# define GET_LOGIN_NAME_MAX() sysconf (_SC_LOGIN_NAME_MAX)
#else
@@ -696,97 +692,43 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (success)
{
struct passwd *p;
- char *malloc_pwtmpbuf = NULL;
- char *pwtmpbuf;
# if defined HAVE_GETPWNAM_R || defined _LIBC
- long int pwbuflenmax = GETPW_R_SIZE_MAX ();
- size_t pwbuflen = pwbuflenmax;
struct passwd pwbuf;
int save = errno;
+ struct scratch_buffer pwtmpbuf;
+ scratch_buffer_init (&pwtmpbuf);
-# ifndef _LIBC
- if (! (0 < pwbuflenmax && pwbuflenmax <= SIZE_MAX))
- /* `sysconf' does not support _SC_GETPW_R_SIZE_MAX.
- Try a moderate value. */
- pwbuflen = 1024;
-# endif
- if (glob_use_alloca (alloca_used, pwbuflen))
- pwtmpbuf = alloca_account (pwbuflen, alloca_used);
- else
- {
- pwtmpbuf = malloc (pwbuflen);
- if (pwtmpbuf == NULL)
- {
- if (__glibc_unlikely (malloc_name))
- free (name);
- retval = GLOB_NOSPACE;
- goto out;
- }
- malloc_pwtmpbuf = pwtmpbuf;
- }
-
- while (getpwnam_r (name, &pwbuf, pwtmpbuf, pwbuflen, &p)
+ while (getpwnam_r (name, &pwbuf,
+ pwtmpbuf.data, pwtmpbuf.length, &p)
!= 0)
{
- size_t newlen;
- bool v;
if (errno != ERANGE)
{
p = NULL;
break;
}
- v = size_add_wrapv (pwbuflen, pwbuflen, &newlen);
- if (!v && malloc_pwtmpbuf == NULL
- && glob_use_alloca (alloca_used, newlen))
- pwtmpbuf = extend_alloca_account (pwtmpbuf, pwbuflen,
- newlen, alloca_used);
- else
+ if (!scratch_buffer_grow (&pwtmpbuf))
{
- char *newp = (v ? NULL
- : realloc (malloc_pwtmpbuf, newlen));
- if (newp == NULL)
- {
- free (malloc_pwtmpbuf);
- if (__glibc_unlikely (malloc_name))
- free (name);
- retval = GLOB_NOSPACE;
- goto out;
- }
- malloc_pwtmpbuf = pwtmpbuf = newp;
+ retval = GLOB_NOSPACE;
+ goto out;
}
- pwbuflen = newlen;
__set_errno (save);
}
# else
- p = getpwnam (name);
+ p = getpwnam (pwtmpbuf.data);
# endif
- if (__glibc_unlikely (malloc_name))
- free (name);
if (p != NULL)
{
- if (malloc_pwtmpbuf == NULL)
- home_dir = p->pw_dir;
- else
+ home_dir = strdup (p->pw_dir);
+ malloc_home_dir = 1;
+ if (home_dir == NULL)
{
- size_t home_dir_len = strlen (p->pw_dir) + 1;
- if (glob_use_alloca (alloca_used, home_dir_len))
- home_dir = alloca_account (home_dir_len,
- alloca_used);
- else
- {
- home_dir = malloc (home_dir_len);
- if (home_dir == NULL)
- {
- free (pwtmpbuf);
- retval = GLOB_NOSPACE;
- goto out;
- }
- malloc_home_dir = 1;
- }
- memcpy (home_dir, p->pw_dir, home_dir_len);
+ scratch_buffer_free (&pwtmpbuf);
+ retval = GLOB_NOSPACE;
+ goto out;
}
}
- free (malloc_pwtmpbuf);
+ scratch_buffer_free (&pwtmpbuf);
}
else
{
@@ -924,59 +866,25 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
/* Look up specific user's home directory. */
{
struct passwd *p;
- char *malloc_pwtmpbuf = NULL;
+ struct scratch_buffer pwtmpbuf;
+ scratch_buffer_init (&pwtmpbuf);
+
# if defined HAVE_GETPWNAM_R || defined _LIBC
- long int buflenmax = GETPW_R_SIZE_MAX ();
- size_t buflen = buflenmax;
- char *pwtmpbuf;
struct passwd pwbuf;
int save = errno;
-# ifndef _LIBC
- if (! (0 <= buflenmax && buflenmax <= SIZE_MAX))
- /* Perhaps 'sysconf' does not support _SC_GETPW_R_SIZE_MAX. Try a
- moderate value. */
- buflen = 1024;
-# endif
- if (glob_use_alloca (alloca_used, buflen))
- pwtmpbuf = alloca_account (buflen, alloca_used);
- else
- {
- pwtmpbuf = malloc (buflen);
- if (pwtmpbuf == NULL)
- {
- nomem_getpw:
- if (__glibc_unlikely (malloc_user_name))
- free (user_name);
- retval = GLOB_NOSPACE;
- goto out;
- }
- malloc_pwtmpbuf = pwtmpbuf;
- }
-
- while (getpwnam_r (user_name, &pwbuf, pwtmpbuf, buflen, &p) != 0)
+ while (getpwnam_r (user_name, &pwbuf,
+ pwtmpbuf.data, pwtmpbuf.length, &p) != 0)
{
- size_t newlen;
- bool v;
if (errno != ERANGE)
{
p = NULL;
break;
}
- v = size_add_wrapv (buflen, buflen, &newlen);
- if (!v && malloc_pwtmpbuf == NULL
- && glob_use_alloca (alloca_used, newlen))
- pwtmpbuf = extend_alloca_account (pwtmpbuf, buflen,
- newlen, alloca_used);
- else
+ if (!scratch_buffer_grow (&pwtmpbuf))
{
- char *newp = v ? NULL : realloc (malloc_pwtmpbuf, newlen);
- if (newp == NULL)
- {
- free (malloc_pwtmpbuf);
- goto nomem_getpw;
- }
- malloc_pwtmpbuf = pwtmpbuf = newp;
+ retval = GLOB_NOSPACE;
+ goto out;
}
__set_errno (save);
}
@@ -1005,7 +913,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
dirname = malloc (home_len + rest_len + 1);
if (dirname == NULL)
{
- free (malloc_pwtmpbuf);
+ scratch_buffer_free (&pwtmpbuf);
retval = GLOB_NOSPACE;
goto out;
}
@@ -1016,18 +924,15 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
dirlen = home_len + rest_len;
dirname_modified = 1;
-
- free (malloc_pwtmpbuf);
}
else
{
- free (malloc_pwtmpbuf);
-
if (flags & GLOB_TILDE_CHECK)
/* We have to regard it as an error if we cannot find the
home directory. */
return GLOB_NOMATCH;
}
+ scratch_buffer_free (&pwtmpbuf);
}
}
# endif /* Not Amiga && not WINDOWS32. */
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 05/17] posix: Consolidate glob implementation
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
` (11 preceding siblings ...)
2017-06-08 21:14 ` [PATCH 14/17] posix: Remove all alloca usage in glob Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 11/17] posix: Remove alloca usage for GLOB_BRACE on glob Adhemerval Zanella
` (3 subsequent siblings)
16 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 UTC (permalink / raw)
To: libc-alpha
This patch consolidates the glob implementation. The main changes are:
* Remove specific defines required for multiple compilation in same
unit (GLOB_ONLY_P, NO_GLOB_PATTERN_P and GLOB_COMPAT_BUILD). To allow
using the same code to build compat version on Linux, extra units are
used instead (oldglob.c).
* Both globfree and GNU extension glob_pattern_p are now on their files.
This simplifies the creation of compat symbol when required.
* Also similar to glob/glob64, a new globfree64 is file is added with an
empty implementatio as default.
* On Linux all implementation now uses a default one with the exception
of alpha (which requires a specific versioning) and s390-32 (which
different than other 32 bits with support for v2.1 symbol does not
add a compat symbol).
* Move i386 olddirent.h header to Linux default directory, since it is
the only header with this name and it is shared among different
architectures (and used on compat glob symbol).
Checked on x86_64-linux-gnu and i686-linux-gnu.
* posix/Makefile (routines): Add globfree, globfree64, and
glob_pattern_p.
* posix/glob.c: Remove GLOB_ONLY_P, GLOB_COMPAT_BUILD, and
NO_GLOB_PATTERN_P define usage.
(globfree): Move to its own file.
(__glob_pattern_type): Likewise.
(__glob_pattern_p): Likewise.
* posix/glob_internal.h: New file.
* posix/glob_pattern_p.c: Likewise.
* posix/globfree.c: Likewise.
* posix/globfree64.c: Likewise.
* sysdeps/gnu/glob64.c: Remove file.
* sysdeps/unix/sysv/linux/Makefile (sysdep_routines): Add oldglob.
* sysdeps/unix/sysv/linux/alpha/Makefile [$(subdir) = posix]
(sysdep_routines): Remove rule.
* sysdeps/unix/sysv/linux/alpha/glob.c: Remove file.
* sysdeps/unix/sysv/linux/arm/glob64.c: Likewise.
* sysdeps/wordsize-64/glob.c: Likewise.
* sysdeps/unix/sysv/linux/m68k/glob64.c: Likewise.
* sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c: Likewise.
* sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c: Likewise.
* sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c: Likewise.
* sysdeps/unix/sysv/linux/wordsize-64/glob64.c: Likewise.
* sysdeps/unix/sysv/linux/x86_64/x32/glob.c: Likewise.
* sysdeps/wordsize-64/glob64.c: Likewise.
* sysdeps/unix/sysv/linux/alpha/glob64.c: New file.
* sysdeps/unix/sysv/linux/alpha/globfree.c: Likewise.
* sysdeps/unix/sysv/linux/glob.c: Likewise.
* sysdeps/unix/sysv/linux/glob64.c: Likewise.
* sysdeps/unix/sysv/linux/globfree.c: Likewise.
* sysdeps/unix/sysv/linux/globfree64.c: Likewise.
* sysdeps/unix/sysv/linux/i386/alphasort64.c: include olddirent.h
using relative path instead of absolute one.
* sysdeps/unix/sysv/linux/i386/getdents64.c: Likewise.
* sysdeps/unix/sysv/linux/i386/readdir64.c: Likewise.
* sysdeps/unix/sysv/linux/i386/readdir64_r.c: Likewise.
* sysdeps/unix/sysv/linux/i386/versionsort64.c: Likewise.
* sysdeps/unix/sysv/linux/i386/olddirent.h: Move to ...
* sysdeps/unix/sysv/linux/olddirent.h: ... here.
* sysdeps/unix/sysv/linux/i386/glob64.c: Move to ...
* sysdeps/unix/sysv/linux/oldglob.c: ... here.
* sysdeps/unix/sysv/linux/i386/glob64.c: Remove file.
* sysdeps/unix/sysv/linux/oldglob.c: New file.
* sysdeps/unix/sysv/linux/s390/s390-32/glob64.c: New file.
* sysdeps/unix/sysv/linux/s390/s390-32/oldglob.c: Likewise.
---
posix/Makefile | 2 +-
posix/glob.c | 82 +---------------------
posix/glob_internal.h | 57 +++++++++++++++
posix/glob_pattern_p.c | 29 ++++++++
posix/globfree.c | 37 ++++++++++
posix/globfree64.c | 27 +++++++
sysdeps/gnu/glob64.c | 25 -------
sysdeps/unix/sysv/linux/Makefile | 2 +-
sysdeps/unix/sysv/linux/alpha/Makefile | 4 --
sysdeps/unix/sysv/linux/alpha/{glob.c => glob64.c} | 11 ---
sysdeps/unix/sysv/linux/alpha/globfree.c | 36 ++++++++++
sysdeps/unix/sysv/linux/arm/glob64.c | 1 -
sysdeps/unix/sysv/linux/glob.c | 23 ++++++
sysdeps/unix/sysv/linux/glob64.c | 55 +++++++++++++++
sysdeps/unix/sysv/linux/globfree.c | 23 ++++++
sysdeps/unix/sysv/linux/globfree64.c | 0
sysdeps/unix/sysv/linux/i386/alphasort64.c | 2 +-
sysdeps/unix/sysv/linux/i386/getdents64.c | 2 +-
sysdeps/unix/sysv/linux/i386/readdir64.c | 2 +-
sysdeps/unix/sysv/linux/i386/readdir64_r.c | 2 +-
sysdeps/unix/sysv/linux/i386/versionsort64.c | 2 +-
sysdeps/unix/sysv/linux/m68k/glob64.c | 1 -
sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c | 1 -
sysdeps/unix/sysv/linux/{i386 => }/olddirent.h | 0
.../unix/sysv/linux/{i386/glob64.c => oldglob.c} | 53 ++++----------
sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c | 1 -
sysdeps/unix/sysv/linux/s390/s390-32/glob64.c | 20 ++++++
sysdeps/unix/sysv/linux/s390/s390-32/oldglob.c | 2 +
sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c | 1 -
sysdeps/unix/sysv/linux/wordsize-64/glob64.c | 2 -
sysdeps/unix/sysv/linux/x86_64/x32/glob.c | 1 -
sysdeps/wordsize-64/glob.c | 8 ---
sysdeps/wordsize-64/glob64.c | 1 -
33 files changed, 330 insertions(+), 185 deletions(-)
create mode 100644 posix/glob_internal.h
create mode 100644 posix/glob_pattern_p.c
create mode 100644 posix/globfree.c
create mode 100644 posix/globfree64.c
delete mode 100644 sysdeps/gnu/glob64.c
rename sysdeps/unix/sysv/linux/alpha/{glob.c => glob64.c} (78%)
create mode 100644 sysdeps/unix/sysv/linux/alpha/globfree.c
delete mode 100644 sysdeps/unix/sysv/linux/arm/glob64.c
create mode 100644 sysdeps/unix/sysv/linux/glob.c
create mode 100644 sysdeps/unix/sysv/linux/glob64.c
create mode 100644 sysdeps/unix/sysv/linux/globfree.c
create mode 100644 sysdeps/unix/sysv/linux/globfree64.c
delete mode 100644 sysdeps/unix/sysv/linux/m68k/glob64.c
delete mode 100644 sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c
rename sysdeps/unix/sysv/linux/{i386 => }/olddirent.h (100%)
rename sysdeps/unix/sysv/linux/{i386/glob64.c => oldglob.c} (61%)
delete mode 100644 sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c
create mode 100644 sysdeps/unix/sysv/linux/s390/s390-32/glob64.c
create mode 100644 sysdeps/unix/sysv/linux/s390/s390-32/oldglob.c
delete mode 100644 sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c
delete mode 100644 sysdeps/unix/sysv/linux/wordsize-64/glob64.c
delete mode 100644 sysdeps/unix/sysv/linux/x86_64/x32/glob.c
delete mode 100644 sysdeps/wordsize-64/glob.c
delete mode 100644 sysdeps/wordsize-64/glob64.c
diff --git a/posix/Makefile b/posix/Makefile
index 1001e54..38cbce9 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -45,7 +45,7 @@ routines := \
getpgid setpgid getpgrp bsd-getpgrp setpgrp getsid setsid \
getresuid getresgid setresuid setresgid \
pathconf sysconf fpathconf \
- glob glob64 fnmatch regex \
+ glob glob64 globfree globfree64 glob_pattern_p fnmatch regex \
confstr \
getopt getopt1 \
sched_setp sched_getp sched_sets sched_gets sched_yield sched_primax \
diff --git a/posix/glob.c b/posix/glob.c
index 250bff1..dc13e26 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -37,8 +37,6 @@
#include <stdio.h> /* Needed on stupid SunOS for assert. */
-#ifndef GLOB_ONLY_P
-
#include <unistd.h>
#if !defined POSIX && defined _POSIX_VERSION
# define POSIX
@@ -90,6 +88,8 @@
#include <fnmatch.h>
+#include "glob_internal.h"
+
#ifdef _SC_GETPW_R_SIZE_MAX
# define GETPW_R_SIZE_MAX() sysconf (_SC_GETPW_R_SIZE_MAX)
#else
@@ -168,8 +168,6 @@ readdir_result_might_be_dir (struct readdir_result d)
D_INO_TO_RESULT (source) \
}
-#endif /* !defined GLOB_ONLY_P */
-
/* Call gl_readdir on STREAM. This macro can be overridden to reduce
type safety if an old interface version needs to be supported. */
#ifndef GL_READDIR
@@ -252,7 +250,6 @@ static bool size_add_wrapv (size_t a, size_t b, size_t *r);
static bool glob_use_alloca (size_t alloca_used, size_t len);
/* We must not compile this function twice. */
-#ifndef GLOB_COMPAT_BUILD
static bool
size_add_wrapv (size_t a, size_t b, size_t *r)
{
@@ -271,7 +268,6 @@ glob_use_alloca (size_t alloca_used, size_t len)
return (!size_add_wrapv (alloca_used, len, &size)
&& __libc_use_alloca (size));
}
-#endif
static int glob_in_dir (const char *pattern, const char *directory,
int flags, int (*errfunc) (const char *, int),
@@ -279,7 +275,6 @@ static int glob_in_dir (const char *pattern, const char *directory,
extern int __glob_pattern_type (const char *pattern, int quote)
attribute_hidden;
-#ifndef GLOB_ONLY_P
static int prefix_array (const char *prefix, char **array, size_t n) __THROWNL;
static int collated_compare (const void *, const void *) __THROWNL;
@@ -308,7 +303,6 @@ next_brace_sub (const char *cp, int flags)
return *cp != '\0' ? cp : NULL;
}
-#endif /* !defined GLOB_ONLY_P */
/* Do glob searching for PATTERN, placing results in PGLOB.
The bits defined above may be set in FLAGS.
@@ -1355,26 +1349,6 @@ libc_hidden_def (glob)
#endif
-#ifndef GLOB_ONLY_P
-
-/* Free storage allocated in PGLOB by a previous `glob' call. */
-void
-globfree (glob_t *pglob)
-{
- if (pglob->gl_pathv != NULL)
- {
- size_t i;
- for (i = 0; i < pglob->gl_pathc; ++i)
- free (pglob->gl_pathv[pglob->gl_offs + i]);
- free (pglob->gl_pathv);
- pglob->gl_pathv = NULL;
- }
-}
-#if defined _LIBC && !defined globfree
-libc_hidden_def (globfree)
-#endif
-
-
/* Do a collated comparison of A and B. */
static int
collated_compare (const void *a, const void *b)
@@ -1451,58 +1425,6 @@ prefix_array (const char *dirname, char **array, size_t n)
}
-/* We must not compile this function twice. */
-#ifndef NO_GLOB_PATTERN_P
-int
-__glob_pattern_type (const char *pattern, int quote)
-{
- const char *p;
- int ret = 0;
-
- for (p = pattern; *p != '\0'; ++p)
- switch (*p)
- {
- case '?':
- case '*':
- return 1;
-
- case '\\':
- if (quote)
- {
- if (p[1] != '\0')
- ++p;
- ret |= 2;
- }
- break;
-
- case '[':
- ret |= 4;
- break;
-
- case ']':
- if (ret & 4)
- return 1;
- break;
- }
-
- return ret;
-}
-
-/* Return nonzero if PATTERN contains any metacharacters.
- Metacharacters can be quoted with backslashes if QUOTE is nonzero. */
-int
-__glob_pattern_p (const char *pattern, int quote)
-{
- return __glob_pattern_type (pattern, quote) == 1;
-}
-# ifdef _LIBC
-weak_alias (__glob_pattern_p, glob_pattern_p)
-# endif
-#endif
-
-#endif /* !defined GLOB_ONLY_P */
-
-
/* 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.
diff --git a/posix/glob_internal.h b/posix/glob_internal.h
new file mode 100644
index 0000000..d989a98
--- /dev/null
+++ b/posix/glob_internal.h
@@ -0,0 +1,57 @@
+/* Shared definition for glob and glob_pattern_p.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef GLOB_INTERNAL_H
+# define GLOB_INTERNAL_H
+
+static inline int
+__glob_pattern_type (const char *pattern, int quote)
+{
+ const char *p;
+ int ret = 0;
+
+ for (p = pattern; *p != '\0'; ++p)
+ switch (*p)
+ {
+ case '?':
+ case '*':
+ return 1;
+
+ case '\\':
+ if (quote)
+ {
+ if (p[1] != '\0')
+ ++p;
+ ret |= 2;
+ }
+ break;
+
+ case '[':
+ ret |= 4;
+ break;
+
+ case ']':
+ if (ret & 4)
+ return 1;
+ break;
+ }
+
+ return ret;
+}
+
+#endif /* GLOB_INTERNAL_H */
diff --git a/posix/glob_pattern_p.c b/posix/glob_pattern_p.c
new file mode 100644
index 0000000..6e451f2
--- /dev/null
+++ b/posix/glob_pattern_p.c
@@ -0,0 +1,29 @@
+/* Return nonzero if PATTERN contains any metacharacters.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <glob.h>
+#include "glob_internal.h"
+
+/* Return nonzero if PATTERN contains any metacharacters.
+ Metacharacters can be quoted with backslashes if QUOTE is nonzero. */
+int
+__glob_pattern_p (const char *pattern, int quote)
+{
+ return __glob_pattern_type (pattern, quote) == 1;
+}
+weak_alias (__glob_pattern_p, glob_pattern_p)
diff --git a/posix/globfree.c b/posix/globfree.c
new file mode 100644
index 0000000..4ebd142
--- /dev/null
+++ b/posix/globfree.c
@@ -0,0 +1,37 @@
+/* Frees the dynamically allocated storage from an earlier call to glob.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <glob.h>
+#include <stdlib.h>
+
+/* Free storage allocated in PGLOB by a previous `glob' call. */
+void
+globfree (glob_t *pglob)
+{
+ if (pglob->gl_pathv != NULL)
+ {
+ size_t i;
+ for (i = 0; i < pglob->gl_pathc; ++i)
+ free (pglob->gl_pathv[pglob->gl_offs + i]);
+ free (pglob->gl_pathv);
+ pglob->gl_pathv = NULL;
+ }
+}
+#ifndef globfree
+libc_hidden_def (globfree)
+#endif
diff --git a/posix/globfree64.c b/posix/globfree64.c
new file mode 100644
index 0000000..cce288e
--- /dev/null
+++ b/posix/globfree64.c
@@ -0,0 +1,27 @@
+/* Frees the dynamically allocated storage from an earlier call to glob.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <glob.h>
+#include <stdlib.h>
+
+/* Free storage allocated in PGLOB by a previous `glob' call. */
+void
+globfree64 (glob64_t *pglob)
+{
+}
+libc_hidden_def (globfree64)
diff --git a/sysdeps/gnu/glob64.c b/sysdeps/gnu/glob64.c
deleted file mode 100644
index 250ff07..0000000
--- a/sysdeps/gnu/glob64.c
+++ /dev/null
@@ -1,25 +0,0 @@
-#include <dirent.h>
-#include <glob.h>
-#include <sys/stat.h>
-
-#define dirent dirent64
-#define __readdir(dirp) __readdir64 (dirp)
-
-#define glob_t glob64_t
-#define glob(pattern, flags, errfunc, pglob) \
- glob64 (pattern, flags, errfunc, pglob)
-#define globfree(pglob) globfree64 (pglob)
-
-#undef stat
-#define stat stat64
-#undef __lstat
-#define __lstat(file, buf) __lxstat64 (_STAT_VER, file, buf)
-
-#define NO_GLOB_PATTERN_P 1
-
-#define COMPILE_GLOB64 1
-
-#include <posix/glob.c>
-
-libc_hidden_def (glob64)
-libc_hidden_def (globfree64)
diff --git a/sysdeps/unix/sysv/linux/Makefile b/sysdeps/unix/sysv/linux/Makefile
index 99b3f9d..68bfc17 100644
--- a/sysdeps/unix/sysv/linux/Makefile
+++ b/sysdeps/unix/sysv/linux/Makefile
@@ -147,7 +147,7 @@ endif
ifeq ($(subdir),posix)
sysdep_headers += bits/initspin.h
-sysdep_routines += sched_getcpu
+sysdep_routines += sched_getcpu oldglob
tests += tst-affinity tst-affinity-pid
diff --git a/sysdeps/unix/sysv/linux/alpha/Makefile b/sysdeps/unix/sysv/linux/alpha/Makefile
index 47bd189..50f4fb1 100644
--- a/sysdeps/unix/sysv/linux/alpha/Makefile
+++ b/sysdeps/unix/sysv/linux/alpha/Makefile
@@ -1,7 +1,3 @@
-ifeq ($(subdir),posix)
-sysdep_routines += oldglob
-endif
-
ifeq ($(subdir),stdlib)
gen-as-const-headers += ucontext-offsets.sym
endif
diff --git a/sysdeps/unix/sysv/linux/alpha/glob.c b/sysdeps/unix/sysv/linux/alpha/glob64.c
similarity index 78%
rename from sysdeps/unix/sysv/linux/alpha/glob.c
rename to sysdeps/unix/sysv/linux/alpha/glob64.c
index 2d7d287..dd86d15 100644
--- a/sysdeps/unix/sysv/linux/alpha/glob.c
+++ b/sysdeps/unix/sysv/linux/alpha/glob64.c
@@ -16,36 +16,25 @@
<http://www.gnu.org/licenses/>. */
#define glob64 __no_glob64_decl
-#define globfree64 __no_globfree64_decl
-#include <sys/types.h>
#include <glob.h>
#include <shlib-compat.h>
/* For Linux/Alpha we have to make the glob symbols versioned. */
#define glob(pattern, flags, errfunc, pglob) \
__new_glob (pattern, flags, errfunc, pglob)
-#define globfree(pglob) \
- __new_globfree (pglob)
/* We need prototypes for these new names. */
extern int __new_glob (const char *__pattern, int __flags,
int (*__errfunc) (const char *, int),
glob_t *__pglob);
-extern void __new_globfree (glob_t *__pglob);
#include <posix/glob.c>
#undef glob
-#undef globfree
#undef glob64
-#undef globfree64
versioned_symbol (libc, __new_glob, glob, GLIBC_2_1);
-versioned_symbol (libc, __new_globfree, globfree, GLIBC_2_1);
libc_hidden_ver (__new_glob, glob)
-libc_hidden_ver (__new_globfree, globfree)
weak_alias (__new_glob, glob64)
-weak_alias (__new_globfree, globfree64)
-libc_hidden_ver (__new_globfree, globfree64)
diff --git a/sysdeps/unix/sysv/linux/alpha/globfree.c b/sysdeps/unix/sysv/linux/alpha/globfree.c
new file mode 100644
index 0000000..9d159f1
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/alpha/globfree.c
@@ -0,0 +1,36 @@
+/* Frees the dynamically allocated storage from an earlier call to glob.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#define globfree64 __no_globfree64_decl
+
+#include <glob.h>
+#include <shlib-compat.h>
+
+#define globfree(pglob) __new_globfree (pglob)
+extern void __new_globfree (glob_t *__pglob);
+
+#include <posix/globfree.c>
+
+#undef globfree
+#undef globfree64
+
+versioned_symbol (libc, __new_globfree, globfree, GLIBC_2_1);
+libc_hidden_ver (__new_globfree, globfree)
+
+weak_alias (__new_globfree, globfree64)
+libc_hidden_ver (__new_globfree, globfree64)
diff --git a/sysdeps/unix/sysv/linux/arm/glob64.c b/sysdeps/unix/sysv/linux/arm/glob64.c
deleted file mode 100644
index 82a9a29..0000000
--- a/sysdeps/unix/sysv/linux/arm/glob64.c
+++ /dev/null
@@ -1 +0,0 @@
-#include <sysdeps/unix/sysv/linux/i386/glob64.c>
diff --git a/sysdeps/unix/sysv/linux/glob.c b/sysdeps/unix/sysv/linux/glob.c
new file mode 100644
index 0000000..153ba8c
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/glob.c
@@ -0,0 +1,23 @@
+/* Find pathnames matching a pattern.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <sys/types.h>
+
+#ifndef __OFF_T_MATCHES_OFF64_T
+# include <posix/glob.c>
+#endif
diff --git a/sysdeps/unix/sysv/linux/glob64.c b/sysdeps/unix/sysv/linux/glob64.c
new file mode 100644
index 0000000..6ff6611
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/glob64.c
@@ -0,0 +1,55 @@
+/* Find pathnames matching a pattern.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <sys/types.h>
+
+#ifdef __OFF_T_MATCHES_OFF64_T
+# define glob64 __no_glob64_decl
+# include <posix/glob.c>
+# undef glob64
+weak_alias (glob, glob64)
+#else
+# include <glob.h>
+# include <dirent.h>
+# include <sys/stat.h>
+
+# define dirent dirent64
+# define __readdir(dirp) __readdir64 (dirp)
+
+# define glob_t glob64_t
+# define glob(pattern, flags, errfunc, pglob) \
+ __glob64 (pattern, flags, errfunc, pglob)
+# define globfree(pglob) globfree64 (pglob)
+
+# undef stat
+# define stat stat64
+
+# define COMPILE_GLOB64 1
+
+# include <posix/glob.c>
+
+# include "shlib-compat.h"
+
+# ifdef GLOB_NO_OLD_VERSION
+strong_alias (__glob64, glob64)
+libc_hidden_def (glob64)
+# else
+versioned_symbol (libc, __glob64, glob64, GLIBC_2_2);
+libc_hidden_ver (__glob64, glob64)
+# endif
+#endif
diff --git a/sysdeps/unix/sysv/linux/globfree.c b/sysdeps/unix/sysv/linux/globfree.c
new file mode 100644
index 0000000..d615510
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/globfree.c
@@ -0,0 +1,23 @@
+/* Frees the dynamically allocated storage from an earlier call to glob.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#define globfree64 __no_globfree64_decl
+#include <posix/globfree.c>
+#undef globfree64
+weak_alias (globfree, globfree64)
+libc_hidden_ver (globfree, globfree64)
diff --git a/sysdeps/unix/sysv/linux/globfree64.c b/sysdeps/unix/sysv/linux/globfree64.c
new file mode 100644
index 0000000..e69de29
diff --git a/sysdeps/unix/sysv/linux/i386/alphasort64.c b/sysdeps/unix/sysv/linux/i386/alphasort64.c
index d5fd47a..04b29b6 100644
--- a/sysdeps/unix/sysv/linux/i386/alphasort64.c
+++ b/sysdeps/unix/sysv/linux/i386/alphasort64.c
@@ -30,7 +30,7 @@ versioned_symbol (libc, __alphasort64, alphasort64, GLIBC_2_2);
#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
-#include <sysdeps/unix/sysv/linux/i386/olddirent.h>
+#include <olddirent.h>
int
__old_alphasort64 (const struct __old_dirent64 **a,
diff --git a/sysdeps/unix/sysv/linux/i386/getdents64.c b/sysdeps/unix/sysv/linux/i386/getdents64.c
index e8b257f..2010bbf 100644
--- a/sysdeps/unix/sysv/linux/i386/getdents64.c
+++ b/sysdeps/unix/sysv/linux/i386/getdents64.c
@@ -28,7 +28,7 @@
#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
-#include <sysdeps/unix/sysv/linux/i386/olddirent.h>
+#include <olddirent.h>
#define __GETDENTS __old_getdents64
#define DIRENT_TYPE struct __old_dirent64
diff --git a/sysdeps/unix/sysv/linux/i386/readdir64.c b/sysdeps/unix/sysv/linux/i386/readdir64.c
index f80b6a7..bd2375f 100644
--- a/sysdeps/unix/sysv/linux/i386/readdir64.c
+++ b/sysdeps/unix/sysv/linux/i386/readdir64.c
@@ -31,7 +31,7 @@ versioned_symbol (libc, __readdir64, readdir64, GLIBC_2_2);
#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
-#include <sysdeps/unix/sysv/linux/i386/olddirent.h>
+#include <olddirent.h>
#define __READDIR attribute_compat_text_section __old_readdir64
#define __GETDENTS __old_getdents64
diff --git a/sysdeps/unix/sysv/linux/i386/readdir64_r.c b/sysdeps/unix/sysv/linux/i386/readdir64_r.c
index 344fd53..8c0262d 100644
--- a/sysdeps/unix/sysv/linux/i386/readdir64_r.c
+++ b/sysdeps/unix/sysv/linux/i386/readdir64_r.c
@@ -31,7 +31,7 @@ versioned_symbol (libc, __readdir64_r, readdir64_r, GLIBC_2_2);
#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
-#include <sysdeps/unix/sysv/linux/i386/olddirent.h>
+#include <olddirent.h>
#define __READDIR_R attribute_compat_text_section __old_readdir64_r
#define __GETDENTS __old_getdents64
diff --git a/sysdeps/unix/sysv/linux/i386/versionsort64.c b/sysdeps/unix/sysv/linux/i386/versionsort64.c
index 3e1c6ea..87f2f95 100644
--- a/sysdeps/unix/sysv/linux/i386/versionsort64.c
+++ b/sysdeps/unix/sysv/linux/i386/versionsort64.c
@@ -30,7 +30,7 @@ versioned_symbol (libc, __versionsort64, versionsort64, GLIBC_2_2);
#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
-#include <sysdeps/unix/sysv/linux/i386/olddirent.h>
+#include <olddirent.h>
int
__old_versionsort64 (const struct __old_dirent64 **a,
diff --git a/sysdeps/unix/sysv/linux/m68k/glob64.c b/sysdeps/unix/sysv/linux/m68k/glob64.c
deleted file mode 100644
index 82a9a29..0000000
--- a/sysdeps/unix/sysv/linux/m68k/glob64.c
+++ /dev/null
@@ -1 +0,0 @@
-#include <sysdeps/unix/sysv/linux/i386/glob64.c>
diff --git a/sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c b/sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c
deleted file mode 100644
index 33918ea..0000000
--- a/sysdeps/unix/sysv/linux/mips/mips64/n64/glob64.c
+++ /dev/null
@@ -1 +0,0 @@
-/* glob64 is in glob.c */
diff --git a/sysdeps/unix/sysv/linux/i386/olddirent.h b/sysdeps/unix/sysv/linux/olddirent.h
similarity index 100%
rename from sysdeps/unix/sysv/linux/i386/olddirent.h
rename to sysdeps/unix/sysv/linux/olddirent.h
diff --git a/sysdeps/unix/sysv/linux/i386/glob64.c b/sysdeps/unix/sysv/linux/oldglob.c
similarity index 61%
rename from sysdeps/unix/sysv/linux/i386/glob64.c
rename to sysdeps/unix/sysv/linux/oldglob.c
index 2dcbe33..af934f9 100644
--- a/sysdeps/unix/sysv/linux/i386/glob64.c
+++ b/sysdeps/unix/sysv/linux/oldglob.c
@@ -1,5 +1,5 @@
-/* Two glob variants with 64-bit support, for dirent64 and __olddirent64.
- Copyright (C) 1998-2017 Free Software Foundation, Inc.
+/* Find pathnames matching a pattern. Compatibility version.
+ Copyright (C) 2017 Free Software Foundation, Inc.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -16,61 +16,32 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <dirent.h>
#include <glob.h>
-#include <sys/stat.h>
-
-#define dirent dirent64
-#define __readdir(dirp) __readdir64 (dirp)
-
-#define glob_t glob64_t
-#define glob(pattern, flags, errfunc, pglob) \
- __glob64 (pattern, flags, errfunc, pglob)
-#define globfree(pglob) globfree64 (pglob)
-
-#undef stat
-#define stat stat64
-#undef __lstat
-#define __lstat(file, buf) __lxstat64 (_STAT_VER, file, buf)
-
-#define NO_GLOB_PATTERN_P 1
-
-#define COMPILE_GLOB64 1
-
-#include <posix/glob.c>
-
#include "shlib-compat.h"
-libc_hidden_def (globfree64)
-
-versioned_symbol (libc, __glob64, glob64, GLIBC_2_2);
-libc_hidden_ver (__glob64, glob64)
-
-#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2)
+#if SHLIB_COMPAT(libc, GLIBC_2_1, GLIBC_2_2) \
+ && !defined(GLOB_NO_OLD_VERSION)
-#include <sysdeps/unix/sysv/linux/i386/olddirent.h>
+#include <olddirent.h>
int __old_glob64 (const char *__pattern, int __flags,
int (*__errfunc) (const char *, int),
glob64_t *__pglob);
-#undef dirent
+#define glob_t glob64_t
+#define globfree(pglob) globfree64 (pglob)
+#undef stat
+#define stat stat64
+
#define dirent __old_dirent64
-#undef GL_READDIR
-# define GL_READDIR(pglob, stream) \
+#define GL_READDIR(pglob, stream) \
((struct __old_dirent64 *) (pglob)->gl_readdir (stream))
-#undef __readdir
#define __readdir(dirp) __old_readdir64 (dirp)
-#undef glob
#define glob(pattern, flags, errfunc, pglob) \
__old_glob64 (pattern, flags, errfunc, pglob)
#define convert_dirent __old_convert_dirent
-#define glob_in_dir __old_glob_in_dir
-#define GLOB_ATTRIBUTE attribute_compat_text_section
-#define GLOB_ONLY_P 1
-
-#define GLOB_COMPAT_BUILD 1
+#define GLOB_ATTRIBUTE attribute_compat_text_section
#include <posix/glob.c>
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c b/sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c
deleted file mode 100644
index 82a9a29..0000000
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/glob64.c
+++ /dev/null
@@ -1 +0,0 @@
-#include <sysdeps/unix/sysv/linux/i386/glob64.c>
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/glob64.c b/sysdeps/unix/sysv/linux/s390/s390-32/glob64.c
new file mode 100644
index 0000000..b00dc54
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/glob64.c
@@ -0,0 +1,20 @@
+/* Find pathnames matching a pattern. S390-32 Linux version.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#define GLOB_NO_OLD_VERSION
+#include <sysdeps/unix/sysv/linux/glob64.c>
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/oldglob.c b/sysdeps/unix/sysv/linux/s390/s390-32/oldglob.c
new file mode 100644
index 0000000..56d7d12
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/oldglob.c
@@ -0,0 +1,2 @@
+#define GLOB_NO_OLD_VERSION
+#include <sysdeps/unix/sysv/linux/oldglob.c>
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c b/sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c
deleted file mode 100644
index 82a9a29..0000000
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/glob64.c
+++ /dev/null
@@ -1 +0,0 @@
-#include <sysdeps/unix/sysv/linux/i386/glob64.c>
diff --git a/sysdeps/unix/sysv/linux/wordsize-64/glob64.c b/sysdeps/unix/sysv/linux/wordsize-64/glob64.c
deleted file mode 100644
index eab7703..0000000
--- a/sysdeps/unix/sysv/linux/wordsize-64/glob64.c
+++ /dev/null
@@ -1,2 +0,0 @@
-/* This file is here so sysdeps/gnu/glob64.c doesn't take precedence. */
-#include <sysdeps/wordsize-64/glob64.c>
diff --git a/sysdeps/unix/sysv/linux/x86_64/x32/glob.c b/sysdeps/unix/sysv/linux/x86_64/x32/glob.c
deleted file mode 100644
index e542747..0000000
--- a/sysdeps/unix/sysv/linux/x86_64/x32/glob.c
+++ /dev/null
@@ -1 +0,0 @@
-#include <sysdeps/wordsize-64/glob.c>
diff --git a/sysdeps/wordsize-64/glob.c b/sysdeps/wordsize-64/glob.c
deleted file mode 100644
index 082faf1..0000000
--- a/sysdeps/wordsize-64/glob.c
+++ /dev/null
@@ -1,8 +0,0 @@
-#define glob64 __no_glob64_decl
-#define globfree64 __no_globfree64_decl
-#include <posix/glob.c>
-#undef glob64
-#undef globfree64
-weak_alias (glob, glob64)
-weak_alias (globfree, globfree64)
-libc_hidden_ver (globfree, globfree64)
diff --git a/sysdeps/wordsize-64/glob64.c b/sysdeps/wordsize-64/glob64.c
deleted file mode 100644
index 33918ea..0000000
--- a/sysdeps/wordsize-64/glob64.c
+++ /dev/null
@@ -1 +0,0 @@
-/* glob64 is in glob.c */
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 08/17] posix: Use char_array for internal glob dirname
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
` (2 preceding siblings ...)
2017-06-08 21:14 ` [PATCH 09/17] posix: Remove glob GET_LOGIN_NAME_MAX usage Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 01/17] posix: Sync glob with gnulib [BZ #1062] Adhemerval Zanella
` (12 subsequent siblings)
16 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 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.
---
posix/glob.c | 294 ++++++++++++++++++++++++++---------------------------------
1 file changed, 131 insertions(+), 163 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index 425d81b..fbb7aa7 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -90,6 +90,7 @@
#include <scratch_buffer.h>
#include "glob_internal.h"
+#include <malloc/char_array.c>
#ifdef _SC_LOGIN_NAME_MAX
# define GET_LOGIN_NAME_MAX() sysconf (_SC_LOGIN_NAME_MAX)
@@ -316,16 +317,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)
{
@@ -333,6 +333,9 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
return -1;
}
+ if (!char_array_init_empty (&dirname))
+ return GLOB_NOSPACE;
+
/* 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] == '/')
@@ -353,12 +356,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;
@@ -416,7 +419,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
pglob->gl_pathc = 0;
pglob->gl_pathv = NULL;
}
- return GLOB_NOSPACE;
+ goto err_nospace;
}
}
@@ -432,6 +435,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
illegal_brace:
if (__glibc_unlikely (!alloca_onealt))
free (onealt);
+ char_array_free (&dirname);
return glob (pattern, flags & ~GLOB_BRACE, errfunc, pglob);
}
@@ -477,7 +481,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 == '}')
@@ -494,9 +499,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;
}
}
@@ -512,14 +518,15 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (filename == NULL)
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
@@ -537,10 +544,12 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
filename = pattern;
#ifdef _AMIGA
- dirname = (char *) "";
+# define CURRENT_FILENAME ""
#else
- dirname = (char *) ".";
+# define CURRENT_FILENAME "."
#endif
+ if (!char_array_set_str (&dirname, CURRENT_FILENAME))
+ goto err_nospace;
dirlen = 0;
}
}
@@ -549,13 +558,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 == ':'
@@ -569,50 +578,48 @@ 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 (filename[0] == '\0'
#if defined __MSDOS__ || defined WINDOWS32
- && dirname[dirlen - 1] != ':'
- && (dirlen < 3 || dirname[dirlen - 2] != ':'
- || dirname[dirlen - 1] != '/')
+ && char_array_pos (&dirname, dirlen - 1) != ':'
+ && (dirlen < 3 || char_array_pos (&dirname, dirlen - 2) != ':'
+ || char_array_pos (&dirname, dirlen - 1) != '/')
#endif
&& dirlen > 1)
/* "pattern/". Expand "pattern", appending slashes. */
{
int orig_flags = flags;
int val;
- 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 -= 2;
+ char_array_crop (&dirname, dirlen);
flags &= ~(GLOB_NOCHECK | GLOB_NOMAGIC);
}
}
- val = glob (dirname, flags | GLOB_MARK, errfunc, pglob);
+ 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));
@@ -629,11 +636,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");
@@ -681,10 +691,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
{
name = malloc (buflen);
if (name == NULL)
- {
- retval = GLOB_NOSPACE;
- goto out;
- }
+ goto err_nospace;
malloc_name = 1;
}
@@ -708,10 +715,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
break;
}
if (!scratch_buffer_grow (&pwtmpbuf))
- {
- retval = GLOB_NOSPACE;
- goto out;
- }
+ goto err_nospace;
__set_errno (save);
}
# else
@@ -724,8 +728,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (home_dir == NULL)
{
scratch_buffer_free (&pwtmpbuf);
- retval = GLOB_NOSPACE;
- goto out;
+ goto err_nospace;
}
}
scratch_buffer_free (&pwtmpbuf);
@@ -754,53 +757,26 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
# endif /* WINDOWS32 */
# endif
/* 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;
}
# if !defined _AMIGA && !defined WINDOWS32
else
{
- 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;
@@ -809,23 +785,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;
@@ -835,8 +811,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 != '\0')
{
@@ -858,7 +834,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
*p = '\0';
}
else
- *((char *) mempcpy (newp, dirname + 1, end_name - dirname))
+ *((char *) mempcpy (newp, dirnamestr + 1, end_name - dirnamestr))
= '\0';
user_name = newp;
}
@@ -898,39 +874,25 @@ 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);
-
- 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);
+ retval = GLOB_NOSPACE;
+ goto out;
}
- *((char *) mempcpy (mempcpy (dirname, p->pw_dir, home_len),
- end_name, rest_len)) = '\0';
- dirlen = home_len + rest_len;
- dirname_modified = 1;
+ dirlen = strlen (p->pw_dir);
+ dirname_modified = true;
}
else
{
if (flags & GLOB_TILDE_CHECK)
/* We have to regard it as an error if we cannot find the
home directory. */
- return GLOB_NOMATCH;
+ {
+ retval = GLOB_NOMATCH;
+ goto out;
+ }
}
scratch_buffer_free (&pwtmpbuf);
}
@@ -948,9 +910,10 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
/* Return the directory if we don't check for error or if it exists. */
if ((flags & GLOB_NOCHECK)
|| (((__builtin_expect (flags & GLOB_ALTDIRFUNC, 0))
- ? ((*pglob->gl_lstat) (dirname, &st) == 0
+ ? ((*pglob->gl_lstat) (char_array_str (&dirname), &st) == 0
&& S_ISDIR (st.st_mode))
- : (__lstat64 (dirname, &st64) == 0 && S_ISDIR (st64.st_mode)))))
+ : (__lstat64 (char_array_str (&dirname), &st64) == 0
+ && S_ISDIR (st64.st_mode)))))
{
size_t newcount = pglob->gl_pathc + pglob->gl_offs;
char **new_gl_pathv;
@@ -958,12 +921,10 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (newcount > SIZE_MAX / sizeof (char *) - 2)
{
nospace:
- if (__glibc_unlikely (malloc_dirname))
- free (dirname);
free (pglob->gl_pathv);
pglob->gl_pathv = NULL;
pglob->gl_pathc = 0;
- return GLOB_NOSPACE;
+ goto err_nospace;
}
new_gl_pathv = realloc (pglob->gl_pathv,
@@ -978,33 +939,32 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
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';
}
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;
+ retval = 0;
+ goto out;
}
/* Not found. */
- return GLOB_NOMATCH;
+ retval = GLOB_NOMATCH;
+ 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
@@ -1017,15 +977,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))
@@ -1039,7 +999,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),
@@ -1047,7 +1007,10 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (status != 0)
{
if ((flags & GLOB_NOCHECK) == 0 || status != GLOB_NOMATCH)
- return status;
+ {
+ retval = status;
+ goto out;
+ }
goto no_matches;
}
@@ -1085,7 +1048,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
globfree (&dirs);
globfree (pglob);
pglob->gl_pathc = 0;
- return status;
+ retval = status;
+ goto out;
}
/* Stick the directory on the front of each name. */
@@ -1096,7 +1060,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
globfree (&dirs);
globfree (pglob);
pglob->gl_pathc = 0;
- return GLOB_NOSPACE;
+ goto err_nospace;
}
}
@@ -1118,7 +1082,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
{
nospace2:
globfree (&dirs);
- return GLOB_NOSPACE;
+ goto err_nospace;
}
new_gl_pathv = realloc (pglob->gl_pathv,
@@ -1133,7 +1097,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
globfree (&dirs);
globfree (pglob);
pglob->gl_pathc = 0;
- return GLOB_NOSPACE;
+ goto err_nospace;
}
++pglob->gl_pathc;
@@ -1145,7 +1109,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
else
{
globfree (&dirs);
- return GLOB_NOMATCH;
+ retval = GLOB_NOMATCH;
+ goto out;
}
}
@@ -1158,7 +1123,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (meta & 2)
{
- 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. */
@@ -1175,12 +1140,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
@@ -1191,19 +1156,20 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
flags = orig_flags;
goto no_matches;
}
- return status;
+ retval = status;
+ goto out;
}
if (dirlen > 0)
{
/* 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;
- return GLOB_NOSPACE;
+ goto err_nospace;
}
}
}
@@ -1228,7 +1194,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
{
globfree (pglob);
pglob->gl_pathc = 0;
- return GLOB_NOSPACE;
+ goto err_nospace;
}
strcpy (&new[len - 2], "/");
pglob->gl_pathv[i] = new;
@@ -1244,10 +1210,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
libc_hidden_def (glob)
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 14/17] posix: Remove all alloca usage in glob
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
` (10 preceding siblings ...)
2017-06-08 21:14 ` [PATCH 12/17] posix: Remove alloca usage on glob dirname Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 05/17] posix: Consolidate glob implementation Adhemerval Zanella
` (4 subsequent siblings)
16 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 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): Remove alloca_used argument.
(glob): Remove alloca_used.
---
posix/glob.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index 62be373..652ae9e 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -58,7 +58,6 @@
#include <dirent.h>
#include <stdlib.h>
#include <string.h>
-#include <alloca.h>
#ifdef _LIBC
# undef strdup
@@ -229,17 +228,11 @@ 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 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);
extern int __glob_pattern_type (const char *pattern, int quote)
attribute_hidden;
@@ -295,7 +288,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)
@@ -972,7 +964,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;
@@ -1079,7 +1071,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
@@ -1256,8 +1248,7 @@ struct globnames_result
The GLOB_APPEND flag is assumed to be set (always appends). */
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)
+ int (*errfunc) (const char *, int), glob_t *pglob)
{
void *stream = NULL;
struct globnames_array globnames;
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 04/17] posix: Adjust glob tests to libsupport
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
` (7 preceding siblings ...)
2017-06-08 21:14 ` [PATCH 16/17] posix: Add common function to get home directory Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 15/17] posix: Use char_array for home_dir in glob Adhemerval Zanella
` (7 subsequent siblings)
16 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 UTC (permalink / raw)
To: libc-alpha
This patch adjust glob tests to use libsupport. It also refactor some
tests to move to a more meaningful file name and to gather similar tests
in a common file:
* move bug-glob3.c tests to tst-glob_basic.c.
* move bug-glob2.c tests to tst-glob_memory.c
* move common definitions to tst-glob_common.c.
Checked on x86_64-linux-gnu.
* posix/Makefile (tests): Remove bug-glob2 and bug-glob3. Add
tst-glob_basic and tst-glob_memory.
* posix/bug-glob3.c: Move to ...
* posix/tst-glob_basic.c: ... here.
* posix/bug-glob2.c: Move to ...
* posix/tst-glob_memory.c: ... here.
* posix/globtest.c: Use libsupport.
* posix/tst-gnuglob.c: Likewise.
* posix/tst-glob_common.c: New file.
---
posix/Makefile | 12 +--
posix/bug-glob3.c | 45 ---------
posix/globtest.c | 163 ++++++++++++++++++-------------
posix/tst-glob_basic.c | 41 ++++++++
posix/tst-glob_common.c | 103 +++++++++++++++++++
posix/{bug-glob2.c => tst-glob_memory.c} | 100 +++----------------
posix/tst-gnuglob.c | 109 +++------------------
7 files changed, 270 insertions(+), 303 deletions(-)
delete mode 100644 posix/bug-glob3.c
create mode 100644 posix/tst-glob_basic.c
create mode 100644 posix/tst-glob_common.c
rename posix/{bug-glob2.c => tst-glob_memory.c} (76%)
diff --git a/posix/Makefile b/posix/Makefile
index 2c0c9f9..1001e54 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -79,7 +79,7 @@ tests := test-errno tstgetopt testfnm runtests runptests \
tst-nice tst-nanosleep tst-regex2 \
transbug tst-rxspencer tst-pcre tst-boost \
bug-ga1 tst-vfork1 tst-vfork2 tst-vfork3 tst-waitid \
- tst-getaddrinfo2 bug-glob2 bug-glob3 tst-sysconf \
+ tst-getaddrinfo2 tst-glob_memory tst-glob_basic tst-sysconf \
tst-execvp1 tst-execvp2 tst-execlp1 tst-execlp2 \
tst-execv1 tst-execv2 tst-execl1 tst-execl2 \
tst-execve1 tst-execve2 tst-execle1 tst-execle2 \
@@ -121,7 +121,7 @@ generated += $(addprefix wordexp-test-result, 1 2 3 4 5 6 7 8 9 10) \
tst-getconf.out \
tst-pcre-mem.out tst-pcre.mtrace tst-boost-mem.out \
tst-boost.mtrace bug-ga2.mtrace bug-ga2-mem.out \
- bug-glob2.mtrace bug-glob2-mem.out tst-vfork3-mem.out \
+ tst-glob_memory.mtrace tst-glob_memory-mem.out tst-vfork3-mem.out \
tst-vfork3.mtrace getconf.speclist tst-fnmatch-mem.out \
tst-fnmatch.mtrace bug-regex36.mtrace
@@ -139,7 +139,7 @@ tests-special += $(objpfx)bug-regex2-mem.out $(objpfx)bug-regex14-mem.out \
$(objpfx)bug-regex21-mem.out $(objpfx)bug-regex31-mem.out \
$(objpfx)tst-rxspencer-no-utf8-mem.out $(objpfx)tst-pcre-mem.out \
$(objpfx)tst-boost-mem.out $(objpfx)tst-getconf.out \
- $(objpfx)bug-glob2-mem.out $(objpfx)tst-vfork3-mem.out \
+ $(objpfx)tst-glob_memory-mem.out $(objpfx)tst-vfork3-mem.out \
$(objpfx)tst-fnmatch-mem.out $(objpfx)bug-regex36-mem.out
xtests-special += $(objpfx)bug-ga2-mem.out
endif
@@ -342,10 +342,10 @@ $(objpfx)bug-ga2-mem.out: $(objpfx)bug-ga2.out
bug-ga2-ENV = MALLOC_TRACE=$(objpfx)bug-ga2.mtrace
-bug-glob2-ENV = MALLOC_TRACE=$(objpfx)bug-glob2.mtrace
+tst-glob_memory-ENV = MALLOC_TRACE=$(objpfx)tst-glob_memory.mtrace
-$(objpfx)bug-glob2-mem.out: $(objpfx)bug-glob2.out
- $(common-objpfx)malloc/mtrace $(objpfx)bug-glob2.mtrace > $@; \
+$(objpfx)tst-glob_memory-mem.out: $(objpfx)tst-glob_memory.out
+ $(common-objpfx)malloc/mtrace $(objpfx)tst-glob_memory.mtrace > $@; \
$(evaluate-test)
$(inst_libexecdir)/getconf: $(inst_bindir)/getconf \
diff --git a/posix/bug-glob3.c b/posix/bug-glob3.c
deleted file mode 100644
index f2fbd70..0000000
--- a/posix/bug-glob3.c
+++ /dev/null
@@ -1,45 +0,0 @@
-#include <glob.h>
-#include <stdio.h>
-#include <string.h>
-
-static int
-do_test (void)
-{
- int result = 0;
- glob_t g;
- g.gl_pathc = 0;
-
- int r = glob ("", 0, NULL, &g);
- if (r != GLOB_NOMATCH)
- {
- puts ("glob (\"\", 0, NULL, &g) did not fail");
- result = 1;
- }
- else if (g.gl_pathc != 0)
- {
- puts ("gl_pathc after glob (\"\", 0, NULL, &g) not zero");
- result = 1;
- }
-
- r = glob ("", GLOB_NOCHECK, NULL, &g);
- if (r != 0)
- {
- puts ("glob (\"\", GLOB_NOCHECK, NULL, &g) did fail");
- result = 1;
- }
- else if (g.gl_pathc != 1)
- {
- puts ("gl_pathc after glob (\"\", GLOB_NOCHECK, NULL, &g) not 1");
- result = 1;
- }
- else if (strcmp (g.gl_pathv[0], "") != 0)
- {
- puts ("gl_pathv[0] after glob (\"\", GLOB_NOCHECK, NULL, &g) not \"\"");
- result = 1;
- }
-
- return result;
-}
-
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
diff --git a/posix/globtest.c b/posix/globtest.c
index 7ffcb91..e5b5891 100644
--- a/posix/globtest.c
+++ b/posix/globtest.c
@@ -1,4 +1,5 @@
-/* Copyright (C) 1997-2017 Free Software Foundation, Inc.
+/* Basic glob tests. It uses an extenal driver script (tst-glob.sh).
+ Copyright (C) 1997-2017 Free Software Foundation, Inc.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -21,77 +22,97 @@
#include <unistd.h>
#include <glob.h>
-int
-main (int argc, char *argv[])
+#include <support/check.h>
+
+#define OPT_BRACE 'b'
+#define OPT_NOCHECK 'c'
+#define OPT_ONLYDIR 'd'
+#define OPT_NOESCAPE 'e'
+#define OPT_ERR 'E'
+#define OPT_NOMAGIC 'g'
+#define OPT_MARK 'm'
+#define OPT_DOOFFS 'o'
+#define OPT_PERIOD 'p'
+#define OPT_QUOTES 'q'
+#define OPT_NOSORT 's'
+#define OPT_TILDE 't'
+#define OPT_TILDE_CHECK 'T'
+
+#define CMDLINE_OPTSTRING "bcdeEgmopqstT"
+
+static int glob_flags = 0;
+static int quotes = 1;
+
+static void
+cmdline_process_function (int c)
{
- int i, j;
- int glob_flags = 0;
- glob_t g;
- int quotes = 1;
+ switch (c)
+ {
+ case OPT_BRACE:
+ glob_flags |= GLOB_BRACE;
+ break;
+ case OPT_NOCHECK:
+ glob_flags |= GLOB_NOCHECK;
+ break;
+ case OPT_ONLYDIR:
+ glob_flags |= GLOB_ONLYDIR;
+ break;
+ case OPT_NOESCAPE:
+ glob_flags |= GLOB_NOESCAPE;
+ break;
+ case OPT_ERR:
+ glob_flags |= GLOB_ERR;
+ break;
+ case OPT_NOMAGIC:
+ glob_flags |= GLOB_NOMAGIC;
+ break;
+ case OPT_MARK:
+ glob_flags |= GLOB_MARK;
+ break;
+ case OPT_DOOFFS:
+ glob_flags |= GLOB_DOOFFS;
+ break;
+ case OPT_PERIOD:
+ glob_flags |= GLOB_PERIOD;
+ break;
+ case OPT_QUOTES:
+ quotes = 0;
+ break;
+ case OPT_NOSORT:
+ glob_flags |= GLOB_NOSORT;
+ break;
+ case OPT_TILDE:
+ glob_flags |= GLOB_TILDE;
+ break;
+ case OPT_TILDE_CHECK:
+ glob_flags |= GLOB_TILDE_CHECK;
+ break;
+ }
+}
- g.gl_offs = 0;
+#define CMDLINE_PROCESS cmdline_process_function
- while ((i = getopt (argc, argv, "bcdeEgmopqstT")) != -1)
- switch(i)
- {
- case 'b':
- glob_flags |= GLOB_BRACE;
- break;
- case 'c':
- glob_flags |= GLOB_NOCHECK;
- break;
- case 'd':
- glob_flags |= GLOB_ONLYDIR;
- break;
- case 'e':
- glob_flags |= GLOB_NOESCAPE;
- break;
- case 'E':
- glob_flags |= GLOB_ERR;
- break;
- case 'g':
- glob_flags |= GLOB_NOMAGIC;
- break;
- case 'm':
- glob_flags |= GLOB_MARK;
- break;
- case 'o':
- glob_flags |= GLOB_DOOFFS;
- g.gl_offs = 1;
- break;
- case 'p':
- glob_flags |= GLOB_PERIOD;
- break;
- case 'q':
- quotes = 0;
- break;
- case 's':
- glob_flags |= GLOB_NOSORT;
- break;
- case 't':
- glob_flags |= GLOB_TILDE;
- break;
- case 'T':
- glob_flags |= GLOB_TILDE_CHECK;
- break;
- default:
- exit (-1);
- }
+static int
+do_test_argv (int argc, char *argv[])
+{
+ int i, j;
+ glob_t g;
- if (optind >= argc || chdir (argv[optind]))
- exit(1);
+ g.gl_offs = glob_flags & GLOB_DOOFFS ? 1 : 0;
- j = optind + 1;
- if (optind + 1 >= argc)
- exit (1);
+ if (argc < 2)
+ FAIL_EXIT1 ("invalid arguments (expecting path for glob)");
+ if (chdir (argv[1]) != 0)
+ FAIL_EXIT1 ("chmod (%s): %m", argv[1]);
- /* Do a glob on each remaining argument. */
- for (j = optind + 1; j < argc; j++) {
- i = glob (argv[j], glob_flags, NULL, &g);
- if (i != 0)
- break;
- glob_flags |= GLOB_APPEND;
- }
+ /* Do a glob on each argument. */
+ for (j = 2; j < argc; j++)
+ {
+ i = glob (argv[j], glob_flags, NULL, &g);
+ if (i != 0)
+ break;
+ glob_flags |= GLOB_APPEND;
+ }
/* Was there an error? */
if (i == GLOB_NOSPACE)
@@ -106,16 +127,18 @@ main (int argc, char *argv[])
if ((glob_flags & GLOB_DOOFFS) && g.gl_pathv[0] == NULL)
g.gl_pathv[0] = (char *) "abc";
- /* Print out the names. Unless otherwise specified, qoute them. */
+ /* Print out the names. Unless otherwise specified, quote them. */
if (g.gl_pathv)
{
for (i = 0; i < g.gl_offs + g.gl_pathc; ++i)
- printf ("%s%s%s\n", quotes ? "`" : "",
- g.gl_pathv[i] ? g.gl_pathv[i] : "(null)",
- quotes ? "'" : "");
+ printf ("%s%s%s\n", quotes ? "`" : "",
+ g.gl_pathv[i] ? g.gl_pathv[i] : "(null)", quotes ? "'" : "");
}
globfree (&g);
return 0;
}
+
+#define TEST_FUNCTION_ARGV do_test_argv
+#include <support/test-driver.c>
diff --git a/posix/tst-glob_basic.c b/posix/tst-glob_basic.c
new file mode 100644
index 0000000..64b95a8
--- /dev/null
+++ b/posix/tst-glob_basic.c
@@ -0,0 +1,41 @@
+/* Basic glob tests.
+ Copyright (C) 2001-2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <glob.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <support/check.h>
+
+static int
+do_test (void)
+{
+ glob_t g;
+ g.gl_pathc = 0;
+
+ TEST_VERIFY_EXIT (glob ("", 0, NULL, &g) == GLOB_NOMATCH);
+ TEST_VERIFY_EXIT (g.gl_pathc == 0);
+
+ TEST_VERIFY_EXIT (glob ("", GLOB_NOCHECK, NULL, &g) == 0);
+ TEST_VERIFY_EXIT (g.gl_pathc == 1);
+ TEST_VERIFY_EXIT (strcmp (g.gl_pathv[0], "") == 0);
+
+ return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/posix/tst-glob_common.c b/posix/tst-glob_common.c
new file mode 100644
index 0000000..b7a66c7
--- /dev/null
+++ b/posix/tst-glob_common.c
@@ -0,0 +1,103 @@
+/* Common glob test definition.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+// #define DEBUG
+#ifdef DEBUG
+# define PRINTF(fmt, args...) \
+ do \
+ { \
+ int save_errno = errno; \
+ printf (fmt, ##args); \
+ errno = save_errno; \
+ } while (0)
+#else
+# define PRINTF(fmt, args...)
+#endif
+
+struct filesystem_t
+{
+ const char *name;
+ int level;
+ int type;
+ mode_t mode;
+};
+
+static long int
+find_file (const char *s, const struct filesystem_t *filesystem,
+ size_t nfiles)
+{
+ int level = 1;
+ long int idx = 0;
+
+ while (s[0] == '/')
+ {
+ if (s[1] == '\0')
+ {
+ s = ".";
+ break;
+ }
+ ++s;
+ }
+
+ if (strcmp (s, ".") == 0)
+ return 0;
+
+ if (s[0] == '.' && s[1] == '/')
+ s += 2;
+
+ while (*s != '\0')
+ {
+ char *endp = strchrnul (s, '/');
+
+ PRINTF ("looking for %.*s, level %d\n", (int) (endp - s), s, level);
+
+ while (idx < nfiles && filesystem[idx].level >= level)
+ {
+ if (filesystem[idx].level == level
+ && memcmp (s, filesystem[idx].name, endp - s) == 0
+ && filesystem[idx].name[endp - s] == '\0')
+ break;
+ ++idx;
+ }
+
+ if (idx == nfiles || filesystem[idx].level < level)
+ {
+ errno = ENOENT;
+ return -1;
+ }
+
+ if (*endp == '\0')
+ return idx + 1;
+
+ if (filesystem[idx].type != DT_DIR
+ && (idx + 1 >= nfiles
+ || filesystem[idx].level >= filesystem[idx + 1].level))
+ {
+ errno = ENOTDIR;
+ return -1;
+ }
+
+ ++idx;
+
+ s = endp + 1;
+ ++level;
+ }
+
+ errno = ENOENT;
+ return -1;
+}
diff --git a/posix/bug-glob2.c b/posix/tst-glob_memory.c
similarity index 76%
rename from posix/bug-glob2.c
rename to posix/tst-glob_memory.c
index 592d957..33961e8 100644
--- a/posix/bug-glob2.c
+++ b/posix/tst-glob_memory.c
@@ -27,18 +27,10 @@
#include <string.h>
#include <sys/stat.h>
-// #define DEBUG
-#ifdef DEBUG
-# define PRINTF(fmt, args...) \
- do \
- { \
- int save_errno = errno; \
- printf (fmt, ##args); \
- errno = save_errno; \
- } while (0)
-#else
-# define PRINTF(fmt, args...)
-#endif
+#include <support/check.h>
+
+#include "tst-glob_common.c"
+
#define LONG_NAME \
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \
@@ -52,13 +44,7 @@
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \
"xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
-static struct
-{
- const char *name;
- int level;
- int type;
- mode_t mode;
-} filesystem[] =
+struct filesystem_t filesystem[] =
{
{ ".", 1, DT_DIR, 0755 },
{ "..", 1, DT_DIR, 0755 },
@@ -79,7 +65,7 @@ static struct
{ "..", 3, DT_DIR, 0755 },
{ "a", 3, DT_REG, 0644 }
};
-#define nfiles (sizeof (filesystem) / sizeof (filesystem[0]))
+static const size_t nfiles = sizeof (filesystem) / sizeof (filesystem[0]);
typedef struct
@@ -91,65 +77,10 @@ typedef struct
} my_DIR;
-static long int
-find_file (const char *s)
-{
- int level = 1;
- long int idx = 0;
-
- if (strcmp (s, ".") == 0)
- return 0;
-
- if (s[0] == '.' && s[1] == '/')
- s += 2;
-
- while (*s != '\0')
- {
- char *endp = strchrnul (s, '/');
-
- PRINTF ("looking for %.*s, level %d\n", (int) (endp - s), s, level);
-
- while (idx < nfiles && filesystem[idx].level >= level)
- {
- if (filesystem[idx].level == level
- && memcmp (s, filesystem[idx].name, endp - s) == 0
- && filesystem[idx].name[endp - s] == '\0')
- break;
- ++idx;
- }
-
- if (idx == nfiles || filesystem[idx].level < level)
- {
- errno = ENOENT;
- return -1;
- }
-
- if (*endp == '\0')
- return idx + 1;
-
- if (filesystem[idx].type != DT_DIR
- && (idx + 1 >= nfiles
- || filesystem[idx].level >= filesystem[idx + 1].level))
- {
- errno = ENOTDIR;
- return -1;
- }
-
- ++idx;
-
- s = endp + 1;
- ++level;
- }
-
- errno = ENOENT;
- return -1;
-}
-
-
static void *
my_opendir (const char *s)
{
- long int idx = find_file (s);
+ long int idx = find_file (s, filesystem, nfiles);
my_DIR *dir;
if (idx == -1)
@@ -241,7 +172,7 @@ my_closedir (void *dir)
static int
my_stat (const char *name, struct stat *st)
{
- long int idx = find_file (name);
+ long int idx = find_file (name, filesystem, nfiles);
if (idx == -1)
{
@@ -275,7 +206,7 @@ init_glob_altdirfuncs (glob_t *pglob)
}
-int
+static int
do_test (void)
{
mtrace ();
@@ -286,10 +217,7 @@ do_test (void)
if (glob ("dir/*able/*", GLOB_ERR | GLOB_ALTDIRFUNC, NULL, &gl)
!= GLOB_ABORTED)
- {
- puts ("glob did not fail with GLOB_ABORTED");
- exit (EXIT_FAILURE);
- }
+ FAIL_EXIT1 ("glob did not fail with GLOB_ABORTED");
globfree (&gl);
@@ -298,10 +226,7 @@ do_test (void)
gl.gl_offs = 3;
if (glob ("dir2/*", GLOB_DOOFFS, NULL, &gl) != GLOB_NOMATCH)
- {
- puts ("glob did not fail with GLOB_NOMATCH");
- exit (EXIT_FAILURE);
- }
+ FAIL_EXIT1 ("glot dit not fail with GLOB_NOMATCH");
globfree (&gl);
@@ -310,5 +235,4 @@ do_test (void)
return 0;
}
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
diff --git a/posix/tst-gnuglob.c b/posix/tst-gnuglob.c
index d753674..f86eef7 100644
--- a/posix/tst-gnuglob.c
+++ b/posix/tst-gnuglob.c
@@ -20,7 +20,6 @@
#include <dirent.h>
#include <errno.h>
-#include <error.h>
#include <glob.h>
#include <mcheck.h>
#include <stdio.h>
@@ -28,21 +27,11 @@
#include <string.h>
#include <sys/stat.h>
+#include <support/check.h>
-// #define DEBUG
-#ifdef DEBUG
-# define PRINTF(fmt, args...) printf (fmt, ##args)
-#else
-# define PRINTF(fmt, args...)
-#endif
-
+#include "tst-glob_common.c"
-static struct
-{
- const char *name;
- int level;
- int type;
-} filesystem[] =
+struct filesystem_t filesystem[] =
{
{ ".", 1, DT_DIR },
{ "..", 1, DT_DIR },
@@ -84,7 +73,7 @@ static struct
{ "..", 4, DT_DIR },
{ "hidden", 4, DT_REG }
};
-#define nfiles (sizeof (filesystem) / sizeof (filesystem[0]))
+static const size_t nfiles = sizeof (filesystem) / sizeof (filesystem[0]);
typedef struct
@@ -96,75 +85,10 @@ typedef struct
} my_DIR;
-static long int
-find_file (const char *s)
-{
- int level = 1;
- long int idx = 0;
-
- while (s[0] == '/')
- {
- if (s[1] == '\0')
- {
- s = ".";
- break;
- }
- ++s;
- }
-
- if (strcmp (s, ".") == 0)
- return 0;
-
- if (s[0] == '.' && s[1] == '/')
- s += 2;
-
- while (*s != '\0')
- {
- char *endp = strchrnul (s, '/');
-
- PRINTF ("looking for %.*s, level %d\n", (int) (endp - s), s, level);
-
- while (idx < nfiles && filesystem[idx].level >= level)
- {
- if (filesystem[idx].level == level
- && memcmp (s, filesystem[idx].name, endp - s) == 0
- && filesystem[idx].name[endp - s] == '\0')
- break;
- ++idx;
- }
-
- if (idx == nfiles || filesystem[idx].level < level)
- {
- errno = ENOENT;
- return -1;
- }
-
- if (*endp == '\0')
- return idx + 1;
-
- if (filesystem[idx].type != DT_DIR
- && (idx + 1 >= nfiles
- || filesystem[idx].level >= filesystem[idx + 1].level))
- {
- errno = ENOTDIR;
- return -1;
- }
-
- ++idx;
-
- s = endp + 1;
- ++level;
- }
-
- errno = ENOENT;
- return -1;
-}
-
-
static void *
my_opendir (const char *s)
{
- long int idx = find_file (s);
+ long int idx = find_file (s, filesystem, nfiles);
my_DIR *dir;
@@ -176,7 +100,7 @@ my_opendir (const char *s)
dir = (my_DIR *) malloc (sizeof (my_DIR));
if (dir == NULL)
- error (EXIT_FAILURE, errno, "cannot allocate directory handle");
+ FAIL_EXIT1 ("cannot allocate directory handle");
dir->level = filesystem[idx].level;
dir->idx = idx;
@@ -247,13 +171,11 @@ my_closedir (void *dir)
static int
my_stat (const char *name, struct stat *st)
{
- long int idx = find_file (name);
+ long int idx = find_file (name, filesystem, nfiles);
if (idx == -1)
- {
- PRINTF ("my_stat (\"%s\", ...) = -1 (%s)\n", name, strerror (errno));
- return -1;
- }
+ FAIL_EXIT1 ("%s (\"%s\", ...) == -1 (%s)", __func__, name,
+ strerror (errno));
memset (st, '\0', sizeof (*st));
@@ -276,7 +198,8 @@ static const char *glob_errstring[] =
[GLOB_ABORTED] = "read error",
[GLOB_NOMATCH] = "no matches found"
};
-#define nglob_errstring (sizeof (glob_errstring) / sizeof (glob_errstring[0]))
+static const size_t nglob_errstring = (sizeof (glob_errstring)
+ / sizeof (glob_errstring[0]));
static const char *
@@ -289,7 +212,7 @@ flagstr (int flags)
"GLOB_ALTDIRFUNC", "GLOB_BRACE", "GLOB_NOMAGIC", "GLOB_TILDE",
"GLOB_ONLYDIR", "GLOB_TILDECHECK"
};
-#define nstrs (sizeof (strs) / sizeof (strs[0]))
+ static const size_t nstrs = (sizeof (strs) / sizeof (strs[0]));
static char buf[100];
char *cp = buf;
int cnt;
@@ -311,7 +234,6 @@ flagstr (int flags)
}
return buf;
-#undef nstrs
}
@@ -378,7 +300,6 @@ test_result (const char *fmt, int flags, glob_t *gl, const char *str[])
return result;
}
-
static int
do_test (void)
{
@@ -416,7 +337,8 @@ do_test (void)
result |= test_result (fmt, flags, &gl, (const char *[]) { c, NULL }); \
else \
printf ("result for glob (\"%s\", %s) = %s\n\n", fmt, flagstr (flags), \
- errstr (errval))
+ errstr (errval)); \
+ globfree (&gl)
test ("*/*/*", 0, 0,
"dir1lev1/dir2lev2/dir1lev3",
@@ -498,5 +420,4 @@ do_test (void)
return result;
}
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 12/17] posix: Remove alloca usage on glob dirname
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
` (9 preceding siblings ...)
2017-06-08 21:14 ` [PATCH 15/17] posix: Use char_array for home_dir in glob Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 14/17] posix: Remove all alloca usage in glob Adhemerval Zanella
` (5 subsequent siblings)
16 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 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.
---
posix/glob.c | 30 ++++++++++--------------------
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index c83feb4..beeb639 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -1272,7 +1272,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;
struct globnames
{
@@ -1312,33 +1311,24 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
struct stat st;
struct_stat64 st64;
} ust;
- 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 ((__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
- ? (*pglob->gl_lstat) (fullname, &ust.st)
- : __lstat64 (fullname, &ust.st64)) == 0)
+ ? (*pglob->gl_lstat) (char_array_str (&fullname), &ust.st)
+ : __lstat64 (char_array_str (&fullname), &ust.st64)) == 0)
/* 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] 31+ messages in thread
* [PATCH 07/17] malloc: Add specialized dynarray for C strings
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
` (4 preceding siblings ...)
2017-06-08 21:14 ` [PATCH 01/17] posix: Sync glob with gnulib [BZ #1062] Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
2017-06-13 9:46 ` Florian Weimer
2017-06-08 21:14 ` [PATCH 06/17] posix: Rewrite to use struct scratch_buffer instead of extend_alloca Adhemerval Zanella
` (10 subsequent siblings)
16 siblings, 1 reply; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 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 = "testi"
char_array_erase (&str, 5);
// str = "123testi"
char_array_prepend_str (&str, "123");
// len = 8
size_t len = char_array_length (&str);
// str = "123testi456"
char_array_append_str (&str, "456");
// str = "123testi789"
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.
* malloc/malloc-internal (check_add_wrapv_size_t): New function.
* malloc/char_array.c: New file.
* malloc/tst-char-array.c: Likewise.
---
malloc/Makefile | 1 +
malloc/char_array.c | 256 +++++++++++++++++++++++++++++++++++++++++++++++
malloc/malloc-internal.h | 14 +++
malloc/tst-char_array.c | 107 ++++++++++++++++++++
4 files changed, 378 insertions(+)
create mode 100644 malloc/char_array.c
create mode 100644 malloc/tst-char_array.c
diff --git a/malloc/Makefile b/malloc/Makefile
index af025cb..098e3c6 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -46,6 +46,7 @@ tests-internal += \
tst-dynarray \
tst-dynarray-fail \
tst-dynarray-at-fail \
+ tst-char_array
ifneq (no,$(have-tunables))
tests += tst-malloc-usable-tunables
diff --git a/malloc/char_array.c b/malloc/char_array.c
new file mode 100644
index 0000000..cce9360
--- /dev/null
+++ b/malloc/char_array.c
@@ -0,0 +1,256 @@
+/* Specialized dynarray for C strings.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+/* This file provides a dynamic C string with an initial stack allocated
+ buffer. Since it is based on dynarray, it provided 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_resize_str (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_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 = "testi";
+ char_array_erase (&str, 5);
+ // str = "123testi";
+ char_array_prepend_str (&str, "123");
+ // len = 8;
+ size_t len = char_array_length (&str);
+ // str = "123testi456";
+ char_array_append_str (&str, "456");
+ // str = "123testi789";
+ char_array_replace_str_pos (&str, 7, "789", 3);
+ */
+
+#define DYNARRAY_STRUCT char_array
+#define DYNARRAY_ELEMENT char
+#define DYNARRAY_PREFIX char_array_
+#define DYNARRAY_ELEMENT_INIT(__e) (*__e = '\0')
+#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>
+
+/* Return a const char for the internal C string handled by 'array'. */
+static const char * __attribute_used__
+char_array_str (struct char_array *array)
+{
+ return char_array_at (array, 0);
+}
+
+/* Return the character at position 'pos' from the char_array 'array'. */
+static char __attribute_used__
+char_array_pos (struct char_array *array, size_t pos)
+{
+ return *char_array_at (array, pos);
+}
+
+static size_t __attribute_used__
+char_array_length (struct char_array *array)
+{
+ /* Exclude the final '\0'. */
+ return array->dynarray_header.used - 1;
+}
+
+/* Copy the contents of string 'str' to char_array 'array', including the
+ final '\0'. */
+static bool __attribute_used__
+char_array_set_str (struct char_array *array, const char *str)
+{
+ size_t size = strlen (str) + 1;
+ if (!char_array_resize (array, size))
+ return false;
+ memcpy (array->dynarray_header.array, str, size);
+ array->dynarray_header.used = size;
+ return true;
+}
+
+/* Copy up 'size' bytes from string 'str' to char_array 'array'. A final
+ '\0' is appended in the char_array. */
+static bool __attribute_used__
+char_array_set_str_size (struct char_array *array, const char *str,
+ size_t size)
+{
+ size_t newsize;
+ if (check_add_wrapv_size_t (size, 1, &newsize)
+ || !char_array_resize (array, newsize))
+ return false;
+ *((char *) mempcpy (array->dynarray_header.array, str, size)) = '\0';
+ array->dynarray_header.used = newsize;
+ return true;
+}
+
+/* Initialize the char_array 'array' and sets it to an empty string (""). */
+static bool __attribute_used__
+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'. */
+static bool __attribute_used__
+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. */
+static bool __attribute_used__
+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);
+}
+
+static bool __attribute_used__
+char_array_is_empty (struct char_array *array)
+{
+ return *char_array_at (array, 0) == '\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. */
+static bool __attribute_used__
+char_array_erase (struct char_array *array, size_t pos)
+{
+ if (pos >= array->dynarray_header.used - 1)
+ return false;
+
+ char *ppos = char_array_at (array, pos);
+ char *lpos = array->dynarray_header.array + array->dynarray_header.used;
+ ptrdiff_t size = lpos - ppos;
+ memmove (ppos, ppos + 1, size);
+ array->dynarray_header.used--;
+ return true;
+}
+
+/* Resize the char_array 'array' to size 'count' maintaining the ending
+ '\0' byte. */
+static bool __attribute_used__
+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[array->dynarray_header.used] = '\0';
+ return true;
+}
+
+/* Prepend the contents of string 'str' to char_array 'array', including the
+ final '\0' byte. */
+static bool __attribute_used__
+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_wrapv_size_t (used, size, &newsize)
+ || !char_array_resize (array, newsize))
+ return false;
+
+ /* Make room for the string and copy it. */
+ memmove (array->dynarray_header.array + size, array->dynarray_header.array,
+ used);
+ memcpy (array->dynarray_header.array, str, size);
+ array->dynarray_header.used = newsize;
+ return true;
+}
+
+/* Append the contents of string 'str' to char_array 'array, including the
+ final '\0' byte. */
+static bool __attribute_used__
+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;
+
+ /* array '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_wrapv_size_t (used + 1, size, &newsize)
+ || !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';
+ array->dynarray_header.used = newsize;
+ return true;
+}
+
+/* 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. */
+static bool __attribute_used__
+char_array_replace_str_pos (struct char_array *array, size_t pos,
+ const char *str, size_t len)
+{
+ if (pos > array->dynarray_header.used)
+ return false;
+
+ size_t newsize;
+ if (check_add_wrapv_size_t (pos, len, &newsize)
+ || check_add_wrapv_size_t (newsize, 1, &newsize)
+ || !char_array_resize (array, newsize))
+ return false;
+
+ char *start = char_array_at (array, pos);
+ *(char *) mempcpy (start, str, len) = '\0';
+ array->dynarray_header.used = newsize;
+ return true;
+}
diff --git a/malloc/malloc-internal.h b/malloc/malloc-internal.h
index dbd801a..3066cd3 100644
--- a/malloc/malloc-internal.h
+++ b/malloc/malloc-internal.h
@@ -101,4 +101,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_wrapv_size_t (size_t a, size_t b, size_t *r)
+{
+#if 5 <= __GNUC__
+ 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..53f9482
--- /dev/null
+++ b/malloc/tst-char_array.c
@@ -0,0 +1,107 @@
+/* Test for char_array.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <string.h>
+
+#include <malloc/char_array.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"));
+ char_array_free (&str);
+ }
+
+ return 0;
+}
+
+#include <support/test-driver.c>
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 09/17] posix: Remove glob GET_LOGIN_NAME_MAX usage
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
2017-06-08 21:13 ` [PATCH 02/17] posix: Allow glob to match dangling symlinks [BZ #866] Adhemerval Zanella
2017-06-08 21:13 ` [PATCH 03/17] support: Add optstring support Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 08/17] posix: Use char_array for internal glob dirname Adhemerval Zanella
` (13 subsequent siblings)
16 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 UTC (permalink / raw)
To: libc-alpha
Current glob implementation allows non limited user name for home
directory construction on GLOB_TILDE case. To accomplish it glob
either construct a name on stack if size are small enough (based
on current alloca_used) value in heap otherwise.
There is no actual login to resize the buffer in case of the resizing
the buffer in case of ERANGE, so a static buffer using glibc default
LOGIN_NAME_MAX is suffice.
Checked on x86_64-linux-gnu.
* posix/glob.c (LOGIN_NAME_MAX): Define if not defined.
(glob): Use static buffer for user_name on getlogin_r.
---
posix/glob.c | 33 +++++----------------------------
1 file changed, 5 insertions(+), 28 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index fbb7aa7..14a502c 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -92,10 +92,8 @@
#include "glob_internal.h"
#include <malloc/char_array.c>
-#ifdef _SC_LOGIN_NAME_MAX
-# define GET_LOGIN_NAME_MAX() sysconf (_SC_LOGIN_NAME_MAX)
-#else
-# define GET_LOGIN_NAME_MAX() (-1)
+#ifndef LOGIN_NAME_MAX
+# define LOGIN_NAME_MAX 256
#endif
\f
static const char *next_brace_sub (const char *begin, int flags) __THROWNL;
@@ -677,25 +675,9 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (home_dir == NULL || home_dir[0] == '\0')
{
int success;
- char *name;
- int malloc_name = 0;
- size_t buflen = GET_LOGIN_NAME_MAX () + 1;
-
- if (buflen == 0)
- /* `sysconf' does not support _SC_LOGIN_NAME_MAX. Try
- a moderate value. */
- buflen = 20;
- if (glob_use_alloca (alloca_used, buflen))
- name = alloca_account (buflen, alloca_used);
- else
- {
- name = malloc (buflen);
- if (name == NULL)
- goto err_nospace;
- malloc_name = 1;
- }
+ char user_name[LOGIN_NAME_MAX];
- success = __getlogin_r (name, buflen) == 0;
+ success = __getlogin_r (user_name, sizeof (user_name)) == 0;
if (success)
{
struct passwd *p;
@@ -705,7 +687,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
struct scratch_buffer pwtmpbuf;
scratch_buffer_init (&pwtmpbuf);
- while (getpwnam_r (name, &pwbuf,
+ while (getpwnam_r (user_name, &pwbuf,
pwtmpbuf.data, pwtmpbuf.length, &p)
!= 0)
{
@@ -733,11 +715,6 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
scratch_buffer_free (&pwtmpbuf);
}
- else
- {
- if (__glibc_unlikely (malloc_name))
- free (name);
- }
}
if (home_dir == NULL || home_dir[0] == '\0')
{
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 15/17] posix: Use char_array for home_dir in glob
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
` (8 preceding siblings ...)
2017-06-08 21:14 ` [PATCH 04/17] posix: Adjust glob tests to libsupport Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 12/17] posix: Remove alloca usage on glob dirname Adhemerval Zanella
` (6 subsequent siblings)
16 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 UTC (permalink / raw)
To: libc-alpha
This patch uses char_array for home directory discovery. It simplifies
the buffer management.
Checked on x86_64-linux-gnu.
* posix/glob.c (glob): Use char_array for home directory.
---
posix/glob.c | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index 652ae9e..4cefdca 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -606,8 +606,15 @@ 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;
+ 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))
+ {
+ retval = GLOB_NOSPACE;
+ goto out;
+ }
# ifdef _AMIGA
if (home_dir == NULL || home_dir[0] == '\0')
home_dir = "SYS:";
@@ -634,7 +641,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
home_dir = "c:/users/default"; /* poor default */
}
# else
- if (home_dir == NULL || home_dir[0] == '\0')
+ if (char_array_is_empty (&home_dir))
{
int success;
char user_name[LOGIN_NAME_MAX];
@@ -667,9 +674,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
# endif
if (p != NULL)
{
- home_dir = strdup (p->pw_dir);
- malloc_home_dir = 1;
- if (home_dir == NULL)
+ if (!char_array_set_str (&home_dir, p->pw_dir))
{
scratch_buffer_free (&pwtmpbuf);
goto err_nospace;
@@ -678,10 +683,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
scratch_buffer_free (&pwtmpbuf);
}
}
- 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)
{
retval = GLOB_NOMATCH;
@@ -689,8 +692,11 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
else
{
- home_dir = (char *) "~"; /* No luck. */
- malloc_home_dir = 0;
+ if (!char_array_set_str (&home_dir, "~"))
+ {
+ retval = GLOB_NOSPACE;
+ goto out;
+ }
}
}
# endif /* WINDOWS32 */
@@ -698,7 +704,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
/* Now construct the full directory. */
if (char_array_pos (&dirname, 1) == '\0')
{
- if (!char_array_set_str (&dirname, home_dir))
+ if (!char_array_set_str (&dirname, char_array_str (&home_dir)))
goto err_nospace;
dirlen = char_array_size (&dirname) - 1;
}
@@ -706,9 +712,11 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
{
/* Replaces '~' by the obtained HOME dir. */
char_array_erase (&dirname, 0);
- if (!char_array_prepend_str (&dirname, home_dir))
+ if (!char_array_prepend_str (&dirname,
+ char_array_str (&home_dir)))
goto err_nospace;
}
+ char_array_free (&home_dir);
dirname_modified = true;
}
# if !defined _AMIGA && !defined WINDOWS32
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 13/17] posix: Use dynarray for globname in glob
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
` (15 preceding siblings ...)
2017-06-08 21:14 ` [PATCH 17/17] posix: More check for overflow allocation " Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
16 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 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_use_alloca): Remove.
(glob_in_dir): Use dynarray for globnames.
---
posix/glob.c | 155 +++++++++++++++--------------------------------------------
1 file changed, 38 insertions(+), 117 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index beeb639..62be373 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -237,33 +237,6 @@ convert_dirent64 (const struct dirent64 *source)
((void) (buf), (void) (len), (void) (newlen), (void) (avar), (void *) 0)
#endif
-/* Set *R = A + B. Return true if the answer is mathematically
- incorrect due to overflow; in this case, *R is the low order
- bits of the correct answer.. */
-
-static bool size_add_wrapv (size_t a, size_t b, size_t *r);
-static bool glob_use_alloca (size_t alloca_used, size_t len);
-
-/* We must not compile this function twice. */
-static bool
-size_add_wrapv (size_t a, size_t b, size_t *r)
-{
-#if 5 <= __GNUC__
- 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);
@@ -1262,6 +1235,20 @@ 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.
@@ -1273,26 +1260,13 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
glob_t *pglob, size_t alloca_used)
{
void *stream = NULL;
- struct globnames
- {
- struct globnames *next;
- size_t count;
- char *name[64];
- };
-#define INITIAL_COUNT sizeof (init_names.name) / sizeof (init_names.name[0])
- struct globnames init_names;
- struct globnames *names = &init_names;
- struct globnames *names_alloca = &init_names;
+ struct globnames_array globnames;
size_t nfound = 0;
- size_t cur = 0;
int meta;
int save;
int result;
- alloca_used += sizeof (init_names);
-
- init_names.next = NULL;
- init_names.count = INITIAL_COUNT;
+ globnames_array_init (&globnames);
meta = __glob_pattern_type (pattern, !(flags & GLOB_NOESCAPE));
if (meta == 0 && (flags & (GLOB_NOCHECK|GLOB_NOMAGIC)))
@@ -1379,30 +1353,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 size = (sizeof (struct globnames)
- + ((count - INITIAL_COUNT)
- * sizeof (char *)));
- if (glob_use_alloca (alloca_used, size))
- newnames = names_alloca
- = alloca_account (size, alloca_used);
- else if ((newnames = malloc (size)) == NULL)
- goto memory_error;
- newnames->count = count;
- newnames->next = names;
- names = newnames;
- cur = 0;
- }
- names->name[cur] = strdup (d.name);
- if (names->name[cur] == NULL)
- goto memory_error;
- ++cur;
- ++nfound;
- if (SIZE_MAX - pglob->gl_offs <= nfound)
- goto memory_error;
+ globnames_array_add (&globnames, strdup (d.name));
+ if (globnames_array_has_failed (&globnames))
+ goto memory_error;
+ nfound++;
}
}
}
@@ -1412,10 +1366,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;
@@ -1436,61 +1393,25 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
if (new_gl_pathv == NULL)
{
memory_error:
- while (1)
- {
- struct globnames *old = names;
- size_t i;
- for (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;
- size_t i;
- for (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);
- }
-
- pglob->gl_pathv = new_gl_pathv;
+ = ret.names[i];
- 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] 31+ messages in thread
* [PATCH 17/17] posix: More check for overflow allocation in glob
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
` (14 preceding siblings ...)
2017-06-08 21:14 ` [PATCH 10/17] posix: User LOGIN_NAME_MAX for all user name in glob Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 13/17] posix: Use dynarray for globname " Adhemerval Zanella
16 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 UTC (permalink / raw)
To: libc-alpha
This patch adds and replace the allocation overflow based using
malloc internal functions check_add_wrapv_size_t and __libc_reallocarray.
Checked on x86_64-linux-gnu.
* posix/glob.c (glob_malloc_incr): New function.
(glob_malloc_incr2): Likewise.
(glob_realloc_incr): Likewise.
(glob): Use glob_{realloc,malloc}_incr{2}.
---
posix/glob.c | 92 +++++++++++++++++++++++++++++++++++++++++-------------------
1 file changed, 63 insertions(+), 29 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index 2b11e45..0de8233 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -302,6 +302,39 @@ get_home_directory (const char *user_name, struct char_array *home_dir)
return retval;
}
+/* Allocate '(size + incr) * typesize' bytes while for overflow on the
+ arithmetic operations. */
+static void *
+glob_malloc_incr (size_t size, size_t incr, size_t typesize)
+{
+ size_t newsize;
+ if (check_add_wrapv_size_t (size, incr, &newsize))
+ return NULL;
+ return __libc_reallocarray (NULL, newsize, typesize);
+}
+
+/* Allocate '(size + incr1 + incr2) * typesize' bytes while for overflow on
+ the arithmetic operations. */
+static void *
+glob_malloc_incr2 (size_t size, size_t incr1, size_t incr2, size_t typesize)
+{
+ size_t newsize;
+ if (check_add_wrapv_size_t (size, incr1, &newsize)
+ || check_add_wrapv_size_t (newsize, incr2, &newsize))
+ return NULL;
+ return __libc_reallocarray (NULL, newsize, typesize);
+}
+
+/* Reallocate '(size + incr1) * typesize' bytes while for overflow on the
+ arithmetic operations. */
+static void *
+glob_realloc_incr (void *old, size_t size, size_t incr, size_t typesize)
+{
+ size_t newsize;
+ if (check_add_wrapv_size_t (size, incr, &newsize))
+ return NULL;
+ return __libc_reallocarray (old, newsize, typesize);
+}
/* Do glob searching for PATTERN, placing results in PGLOB.
The bits defined above may be set in FLAGS.
@@ -356,11 +389,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
{
size_t i;
- if (pglob->gl_offs >= ~((size_t) 0) / sizeof (char *))
- goto err_nospace;
-
- pglob->gl_pathv = (char **) malloc ((pglob->gl_offs + 1)
- * sizeof (char *));
+ pglob->gl_pathv = glob_malloc_incr (pglob->gl_offs, 1,
+ sizeof (char *));
if (pglob->gl_pathv == NULL)
goto err_nospace;
@@ -816,10 +846,11 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
: (__lstat64 (char_array_str (&dirname), &st64) == 0
&& S_ISDIR (st64.st_mode)))))
{
- size_t newcount = pglob->gl_pathc + pglob->gl_offs;
char **new_gl_pathv;
+ size_t newcount;
- if (newcount > SIZE_MAX / sizeof (char *) - 2)
+ if (check_add_wrapv_size_t (pglob->gl_pathc, pglob->gl_offs,
+ &newcount))
{
nospace:
free (pglob->gl_pathv);
@@ -828,8 +859,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
goto err_nospace;
}
- new_gl_pathv = realloc (pglob->gl_pathv,
- (newcount + 2) * sizeof (char *));
+ new_gl_pathv = glob_realloc_incr (pglob->gl_pathv, newcount, 2,
+ sizeof (char *));
if (new_gl_pathv == NULL)
goto nospace;
pglob->gl_pathv = new_gl_pathv;
@@ -837,9 +868,12 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (flags & GLOB_MARK)
{
char *p;
- pglob->gl_pathv[newcount] = malloc (dirlen + 2);
+
+ pglob->gl_pathv[newcount] = glob_malloc_incr (dirlen, 2,
+ sizeof (char));
if (pglob->gl_pathv[newcount] == NULL)
goto nospace;
+
p = mempcpy (pglob->gl_pathv[newcount],
char_array_str (&dirname), dirlen);
p[0] = '/';
@@ -976,18 +1010,19 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
/* No matches. */
if (flags & GLOB_NOCHECK)
{
- size_t newcount = pglob->gl_pathc + pglob->gl_offs;
+ size_t newcount;
char **new_gl_pathv;
- if (newcount > SIZE_MAX / sizeof (char *) - 2)
+ if (check_add_wrapv_size_t (pglob->gl_pathc, pglob->gl_offs,
+ &newcount))
{
nospace2:
globfree (&dirs);
goto err_nospace;
}
- new_gl_pathv = realloc (pglob->gl_pathv,
- (newcount + 2) * sizeof (char *));
+ new_gl_pathv = glob_realloc_incr (pglob->gl_pathv, newcount, 2,
+ sizeof (char *));
if (new_gl_pathv == NULL)
goto nospace2;
pglob->gl_pathv = new_gl_pathv;
@@ -1089,15 +1124,16 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
: (__lstat64 (pglob->gl_pathv[i], &st64) == 0
&& (S_ISDIR (st64.st_mode) || S_ISLNK (st64.st_mode)))))
{
- size_t len = strlen (pglob->gl_pathv[i]) + 2;
- char *new = realloc (pglob->gl_pathv[i], len);
+ size_t len = strlen (pglob->gl_pathv[i]);
+ char *new = glob_realloc_incr (pglob->gl_pathv[i], len, 2,
+ sizeof (char));
if (new == NULL)
{
globfree (pglob);
pglob->gl_pathc = 0;
goto err_nospace;
}
- strcpy (&new[len - 2], "/");
+ strcpy (&new[len], "/");
pglob->gl_pathv[i] = new;
}
}
@@ -1182,7 +1218,7 @@ prefix_array (const char *dirname, char **array, size_t n)
for (i = 0; i < n; ++i)
{
size_t eltlen = strlen (array[i]) + 1;
- char *new = malloc (dirlen + 1 + eltlen);
+ char *new = glob_malloc_incr2 (dirlen, 1, eltlen, sizeof (char));
if (new == NULL)
{
while (i > 0)
@@ -1190,11 +1226,10 @@ prefix_array (const char *dirname, char **array, size_t n)
return 1;
}
- {
- char *endp = mempcpy (new, dirname, dirlen);
- *endp++ = DIRSEP_CHAR;
- mempcpy (endp, array[i], eltlen);
- }
+ char *endp = mempcpy (new, dirname, dirlen);
+ *endp++ = DIRSEP_CHAR;
+ mempcpy (endp, array[i], eltlen);
+
free (array[i]);
array[i] = new;
}
@@ -1345,16 +1380,15 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
if (nfound != 0)
{
char **new_gl_pathv;
+ size_t newlen;
result = 0;
- if (SIZE_MAX / sizeof (char *) - pglob->gl_pathc
- < pglob->gl_offs + nfound + 1)
+ if (check_add_wrapv_size_t (pglob->gl_pathc, pglob->gl_offs, &newlen)
+ || check_add_wrapv_size_t (newlen, nfound, &newlen)
+ || check_add_wrapv_size_t (newlen, 1, &newlen))
goto memory_error;
- new_gl_pathv
- = realloc (pglob->gl_pathv,
- (pglob->gl_pathc + pglob->gl_offs + nfound + 1)
- * sizeof (char *));
+ new_gl_pathv = realloc (pglob->gl_pathv, newlen * sizeof (char *));
if (new_gl_pathv == NULL)
{
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 11/17] posix: Remove alloca usage for GLOB_BRACE on glob
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
` (12 preceding siblings ...)
2017-06-08 21:14 ` [PATCH 05/17] posix: Consolidate glob implementation Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 10/17] posix: User LOGIN_NAME_MAX for all user name in glob Adhemerval Zanella
` (2 subsequent siblings)
16 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 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.
---
posix/glob.c | 63 ++++++++++++++++++++++++++++++------------------------------
1 file changed, 32 insertions(+), 31 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index 9e3050a..c83feb4 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -397,44 +397,32 @@ 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
+ struct char_array onealt;
+
+ /* We know the prefix for all sub-patterns. */
+ ptrdiff_t onealtlen = begin - pattern;
+ if (!char_array_init_str_size (&onealt, pattern, onealtlen))
{
- onealt = malloc (pattern_len);
- if (onealt == NULL)
+ if (!(flags & GLOB_APPEND))
{
- if (!(flags & GLOB_APPEND))
- {
- pglob->gl_pathc = 0;
- pglob->gl_pathv = NULL;
- }
- goto err_nospace;
+ pglob->gl_pathc = 0;
+ pglob->gl_pathv = NULL;
}
+ goto err_nospace;
}
- /* We know the prefix for all sub-patterns. */
- alt_start = mempcpy (onealt, pattern, begin - pattern);
-
/* Find the first sub-pattern and at the same time find the
rest after the closing brace. */
next = next_brace_sub (begin + 1, flags);
if (next == NULL)
{
/* It is an invalid expression. */
- illegal_brace:
- if (__glibc_unlikely (!alloca_onealt))
- free (onealt);
- char_array_free (&dirname);
- return glob (pattern, flags & ~GLOB_BRACE, errfunc, pglob);
+ char_array_free (&onealt);
+ goto illegal_brace;
}
/* Now find the end of the whole brace expression. */
@@ -443,8 +431,11 @@ 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);
+ goto illegal_brace;
+ }
}
/* Please note that we now can be sure the brace expression
is well-formed. */
@@ -463,17 +454,24 @@ 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);
+ retval = GLOB_NOSPACE;
+ goto out;
+ }
- result = glob (onealt,
+ result = glob (char_array_str (&onealt),
((flags & ~(GLOB_NOCHECK | GLOB_NOMAGIC))
| GLOB_APPEND), errfunc, pglob);
/* 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);
@@ -492,8 +490,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. */
@@ -1180,6 +1177,10 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
err_nospace:
char_array_free (&dirname);
return GLOB_NOSPACE;
+
+ illegal_brace:
+ char_array_free (&dirname);
+ return glob (pattern, flags & ~GLOB_BRACE, errfunc, pglob);
}
#if defined _LIBC && !defined glob
libc_hidden_def (glob)
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 10/17] posix: User LOGIN_NAME_MAX for all user name in glob
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
` (13 preceding siblings ...)
2017-06-08 21:14 ` [PATCH 11/17] posix: Remove alloca usage for GLOB_BRACE on glob Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 17/17] posix: More check for overflow allocation " Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 13/17] posix: Use dynarray for globname " Adhemerval Zanella
16 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 UTC (permalink / raw)
To: libc-alpha
This patch limits all user name obtained for GLOB_TILDE to max of
LOGIN_NAME_MAX (256 on glibc) and remove all stack/malloc buffer
handling boilerplate.
Checked on x86_64-linux-gnu.
* posix/glob.c (glob): Remove alloca usage on user_name for
GLOB_TILDE.
---
posix/glob.c | 35 +++++++++++------------------------
1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index 14a502c..9e3050a 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -754,8 +754,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
{
char *dirnamestr = char_array_at (&dirname, 0);
char *end_name = strchr (dirnamestr, '/');
- char *user_name;
- int malloc_user_name = 0;
+ char user_name[LOGIN_NAME_MAX];
char *unescape = NULL;
if (!(flags & GLOB_NOESCAPE))
@@ -770,26 +769,14 @@ 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;
+ strncpy (user_name, dirnamestr + 1, LOGIN_NAME_MAX - 1);
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);
+ ptrdiff_t name_len = unescape - dirnamestr - 1;
+ name_len = MIN (name_len, LOGIN_NAME_MAX - 1);
+ char *p = mempcpy (user_name, dirnamestr + 1, name_len);
char *q = unescape;
while (*q != '\0')
{
@@ -811,9 +798,12 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
*p = '\0';
}
else
- *((char *) mempcpy (newp, dirnamestr + 1, end_name - dirnamestr))
- = '\0';
- user_name = newp;
+ {
+ ptrdiff_t name_len = end_name - dirnamestr;
+ name_len = MIN (name_len, LOGIN_NAME_MAX - 1);
+ *((char *) mempcpy (user_name, dirnamestr + 1, name_len))
+ = '\0';
+ }
}
/* Look up specific user's home directory. */
@@ -845,9 +835,6 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
p = getpwnam (user_name);
# endif
- if (__glibc_unlikely (malloc_user_name))
- free (user_name);
-
/* If we found a home directory use this. */
if (p != NULL)
{
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 16/17] posix: Add common function to get home directory
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
` (6 preceding siblings ...)
2017-06-08 21:14 ` [PATCH 06/17] posix: Rewrite to use struct scratch_buffer instead of extend_alloca Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 04/17] posix: Adjust glob tests to libsupport Adhemerval Zanella
` (8 subsequent siblings)
16 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 UTC (permalink / raw)
To: libc-alpha
This patch adds a common function to get the full home directory
from a user. No functional changes expected.
Checked on x86_64-linux-gnu.
* posix/glob.c (get_home_directory): New function.
(glob): Use get_home_directory.
---
posix/glob.c | 145 +++++++++++++++++++++++------------------------------------
1 file changed, 56 insertions(+), 89 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index 4cefdca..2b11e45 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -264,6 +264,44 @@ next_brace_sub (const char *cp, int flags)
return *cp != '\0' ? cp : NULL;
}
+/* Obtain the full home directory path from user 'user_name' and writes it
+ on char_array 'home_dir'. */
+static bool
+get_home_directory (const char *user_name, struct char_array *home_dir)
+{
+ struct passwd *p;
+#if defined HAVE_GETPWNAM_R || defined _LIBC
+ struct passwd pwbuf;
+ int save = errno;
+ struct scratch_buffer pwtmpbuf;
+ scratch_buffer_init (&pwtmpbuf);
+
+ while (getpwnam_r (user_name, &pwbuf, pwtmpbuf.data, pwtmpbuf.length, &p)
+ != 0)
+ {
+ if (errno != ERANGE)
+ {
+ p = NULL;
+ break;
+ }
+ if (!scratch_buffer_grow (&pwtmpbuf))
+ return false;
+ __set_errno (save);
+ }
+#else
+ p = getpwnam (pwtmpbuf.data);
+#endif
+
+ bool retval = false;
+ if (p != NULL)
+ {
+ if (char_array_set_str (home_dir, p->pw_dir))
+ retval = true;
+ }
+ scratch_buffer_free (&pwtmpbuf);
+ return retval;
+}
+
/* Do glob searching for PATTERN, placing results in PGLOB.
The bits defined above may be set in FLAGS.
@@ -649,38 +687,8 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
success = __getlogin_r (user_name, sizeof (user_name)) == 0;
if (success)
{
- struct passwd *p;
-# if defined HAVE_GETPWNAM_R || defined _LIBC
- struct passwd pwbuf;
- int save = errno;
- struct scratch_buffer pwtmpbuf;
- scratch_buffer_init (&pwtmpbuf);
-
- while (getpwnam_r (user_name, &pwbuf,
- pwtmpbuf.data, pwtmpbuf.length, &p)
- != 0)
- {
- if (errno != ERANGE)
- {
- p = NULL;
- break;
- }
- if (!scratch_buffer_grow (&pwtmpbuf))
- goto err_nospace;
- __set_errno (save);
- }
-# else
- p = getpwnam (pwtmpbuf.data);
-# endif
- if (p != NULL)
- {
- if (!char_array_set_str (&home_dir, p->pw_dir))
- {
- scratch_buffer_free (&pwtmpbuf);
- goto err_nospace;
- }
- }
- scratch_buffer_free (&pwtmpbuf);
+ if (!get_home_directory (user_name, &home_dir))
+ goto err_nospace;
}
}
if (char_array_is_empty (&home_dir))
@@ -693,10 +701,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
else
{
if (!char_array_set_str (&home_dir, "~"))
- {
- retval = GLOB_NOSPACE;
- goto out;
- }
+ goto err_nospace;
}
}
# endif /* WINDOWS32 */
@@ -777,59 +782,21 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
/* Look up specific user's home directory. */
- {
- struct passwd *p;
- struct scratch_buffer pwtmpbuf;
- scratch_buffer_init (&pwtmpbuf);
-
-# if defined HAVE_GETPWNAM_R || defined _LIBC
- struct passwd pwbuf;
- int save = errno;
-
- while (getpwnam_r (user_name, &pwbuf,
- pwtmpbuf.data, pwtmpbuf.length, &p) != 0)
- {
- if (errno != ERANGE)
- {
- p = NULL;
- break;
- }
- if (!scratch_buffer_grow (&pwtmpbuf))
- {
- retval = GLOB_NOSPACE;
- goto out;
- }
- __set_errno (save);
- }
-# else
- p = getpwnam (user_name);
-# endif
-
- /* If we found a home directory use this. */
- if (p != NULL)
- {
- if (!char_array_set_str (&dirname, p->pw_dir))
- {
- scratch_buffer_free (&pwtmpbuf);
- retval = GLOB_NOSPACE;
- goto out;
- }
-
- dirlen = strlen (p->pw_dir);
- dirname_modified = true;
- }
- else
- {
- if (flags & GLOB_TILDE_CHECK)
- /* We have to regard it as an error if we cannot find the
- home directory. */
- {
- retval = GLOB_NOMATCH;
- goto out;
- }
- }
- scratch_buffer_free (&pwtmpbuf);
- }
+ if (get_home_directory (user_name, &dirname))
+ {
+ dirlen = char_array_size (&dirname) - 1;
+ dirname_modified = true;
+ }
+ else
+ {
+ if (flags & GLOB_TILDE_CHECK)
+ /* We have to regard it as an error if we cannot find the
+ home directory. */
+ {
+ retval = GLOB_NOMATCH;
+ goto out;
+ }
+ }
}
# endif /* Not Amiga && not WINDOWS32. */
}
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 01/17] posix: Sync glob with gnulib [BZ #1062]
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
` (3 preceding siblings ...)
2017-06-08 21:14 ` [PATCH 08/17] posix: Use char_array for internal glob dirname Adhemerval Zanella
@ 2017-06-08 21:14 ` Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 07/17] malloc: Add specialized dynarray for C strings Adhemerval Zanella
` (11 subsequent siblings)
16 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-08 21:14 UTC (permalink / raw)
To: libc-alpha
This patch syncs posix/glob.c implementation with gnulib version
1540f34. The main differences to gnulib code:
1. Commit 44c637c (Properly initialize glob structure with
GLOB_BRACE|GLOB_DOOFFS) which fixes BZ# 20707.
2. No inclusion of flexmember.h header and its usage on glob.
The code is meant to be rewritten and header is unrequired in
next patch in this set.
3. An additional define (GLOB_COMPAT_BUILD) to avoid building
size_and_wrapv and gblo_use_alloca twice on some configurations
(i368 compat code) due multiple inclusion.
The main changes are:
- Header organization mostly due gnulib requirements. It leads
to some simplification and less conditional includes.
- Use of glob_use_alloca with wraps up __libc_use_alloca with
saturated math for the total size calculation.
- Simplify some size allocation overflow calculation.
- Some fixed on non supported glibc systems.
- Some comments adjustments.
The changes does not alter current glob internal semantic. I also
added a missing globfree on posix/globtest.c (it helps silence
some valgrind or other memory profilers).
[BZ #1062]
* posix/glob.c: Sync with gnulib.
* posix/globtest.c (main): Add final globfree.
* sysdeps/unix/sysv/linux/i386/glob64.c (GLOB_COMPAT_BUILD):
Define.
---
posix/glob.c | 473 ++++++++++++++++++++--------------
posix/globtest.c | 3 +
sysdeps/unix/sysv/linux/i386/glob64.c | 2 +
3 files changed, 285 insertions(+), 193 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index c653809..3c6b033 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -15,7 +15,10 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#ifdef HAVE_CONFIG_H
+#ifndef _LIBC
+/* Don't use __attribute__ __nonnull__ in this compilation unit. Otherwise gcc
+ optimizes away the pattern == NULL || pglob == NULL tests below. */
+# define _GL_ARG_NONNULL(params)
# include <config.h>
#endif
@@ -34,22 +37,19 @@
#include <stdio.h> /* Needed on stupid SunOS for assert. */
-#if !defined _LIBC || !defined GLOB_ONLY_P
-#if defined HAVE_UNISTD_H || defined _LIBC
-# include <unistd.h>
-# ifndef POSIX
-# ifdef _POSIX_VERSION
-# define POSIX
-# endif
-# endif
+#ifndef GLOB_ONLY_P
+
+#include <unistd.h>
+#if !defined POSIX && defined _POSIX_VERSION
+# define POSIX
#endif
-#include <pwd.h>
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+# define WINDOWS32
+#endif
-#if defined HAVE_STDINT_H || defined _LIBC
-# include <stdint.h>
-#elif !defined UINTPTR_MAX
-# define UINTPTR_MAX (~((size_t) 0))
+#ifndef WINDOWS32
+# include <pwd.h>
#endif
#include <errno.h>
@@ -57,24 +57,7 @@
# define __set_errno(val) errno = (val)
#endif
-#if defined HAVE_DIRENT_H || defined __GNU_LIBRARY__
-# include <dirent.h>
-#else
-# define dirent direct
-# ifdef HAVE_SYS_NDIR_H
-# include <sys/ndir.h>
-# endif
-# ifdef HAVE_SYS_DIR_H
-# include <sys/dir.h>
-# endif
-# ifdef HAVE_NDIR_H
-# include <ndir.h>
-# endif
-# ifdef HAVE_VMSDIR_H
-# include "vmsdir.h"
-# endif /* HAVE_VMSDIR_H */
-#endif
-
+#include <dirent.h>
#include <stdlib.h>
#include <string.h>
#include <alloca.h>
@@ -93,17 +76,16 @@
# endif
# define struct_stat64 struct stat64
#else /* !_LIBC */
-# include "getlogin_r.h"
-# include "mempcpy.h"
-# include "stat-macros.h"
-# include "strdup.h"
-# define __stat64(fname, buf) stat (fname, buf)
-# define struct_stat64 struct stat
-# define __stat(fname, buf) stat (fname, buf)
-# define __alloca alloca
-# define __readdir readdir
-# define __readdir64 readdir64
-# define __glob_pattern_p glob_pattern_p
+# define __getlogin_r(buf, len) getlogin_r (buf, len)
+# define __stat64(fname, buf) stat (fname, buf)
+# define __fxstatat64(_, d, f, st, flag) fstatat (d, f, st, flag)
+# define struct_stat64 struct stat
+# ifndef __MVS__
+# define __alloca alloca
+# endif
+# define __readdir readdir
+# define __glob_pattern_p glob_pattern_p
+# define COMPILE_GLOB64
#endif /* _LIBC */
#include <fnmatch.h>
@@ -186,7 +168,7 @@ readdir_result_might_be_dir (struct readdir_result d)
D_INO_TO_RESULT (source) \
}
-#endif /* !defined _LIBC || !defined GLOB_ONLY_P */
+#endif /* !defined GLOB_ONLY_P */
/* Call gl_readdir on STREAM. This macro can be overridden to reduce
type safety if an old interface version needs to be supported. */
@@ -230,13 +212,74 @@ convert_dirent64 (const struct dirent64 *source)
# define attribute_hidden
#endif
+#ifndef __attribute_noinline__
+# if __GNUC__ < 3 || (__GNUC__ == 3 && __GNUC_MINOR__ < 1)
+# define __attribute_noinline__ /* Ignore */
+#else
+# define __attribute_noinline__ __attribute__ ((__noinline__))
+# endif
+#endif
+
+#ifndef __glibc_unlikely
+# define __glibc_unlikely(expr) __builtin_expect (expr, 0)
+#endif
+
+#ifndef _LIBC
+/* The results of opendir() in this file are not used with dirfd and fchdir,
+ and we do not leak fds to any single-threaded code that could use stdio,
+ therefore save some unnecessary recursion in fchdir.c and opendir_safer.c.
+ FIXME - if the kernel ever adds support for multi-thread safety for
+ avoiding standard fds, then we should use opendir_safer. */
+# ifdef GNULIB_defined_opendir
+# undef opendir
+# endif
+# ifdef GNULIB_defined_closedir
+# undef closedir
+# endif
+
+/* Just use malloc. */
+# define __libc_use_alloca(n) false
+# define alloca_account(len, avar) ((void) (len), (void) (avar), (void *) 0)
+# define extend_alloca_account(buf, len, newlen, avar) \
+ ((void) (buf), (void) (len), (void) (newlen), (void) (avar), (void *) 0)
+#endif
+
+/* Set *R = A + B. Return true if the answer is mathematically
+ incorrect due to overflow; in this case, *R is the low order
+ bits of the correct answer.. */
+
+static bool size_add_wrapv (size_t a, size_t b, size_t *r);
+static bool glob_use_alloca (size_t alloca_used, size_t len);
+
+/* We must not compile this function twice. */
+#ifndef GLOB_COMPAT_BUILD
+static bool
+size_add_wrapv (size_t a, size_t b, size_t *r)
+{
+#if 5 <= __GNUC__
+ 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));
+}
+#endif
+
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);
extern int __glob_pattern_type (const char *pattern, int quote)
attribute_hidden;
-#if !defined _LIBC || !defined GLOB_ONLY_P
+#ifndef GLOB_ONLY_P
static int prefix_array (const char *prefix, char **array, size_t n) __THROWNL;
static int collated_compare (const void *, const void *) __THROWNL;
@@ -265,16 +308,16 @@ next_brace_sub (const char *cp, int flags)
return *cp != '\0' ? cp : NULL;
}
-#endif /* !defined _LIBC || !defined GLOB_ONLY_P */
+#endif /* !defined GLOB_ONLY_P */
/* Do glob searching for PATTERN, placing results in PGLOB.
The bits defined above may be set in FLAGS.
If a directory cannot be opened or read and ERRFUNC is not nil,
it is called with the pathname that caused the error, and the
- `errno' value from the failing call; if it returns non-zero
- `glob' returns GLOB_ABORTED; if it returns zero, the error is ignored.
+ 'errno' value from the failing call; if it returns non-zero
+ 'glob' returns GLOB_ABORTED; if it returns zero, the error is ignored.
If memory cannot be allocated for PGLOB, GLOB_NOSPACE is returned.
- Otherwise, `glob' returns zero. */
+ Otherwise, 'glob' returns zero. */
int
#ifdef GLOB_ATTRIBUTE
GLOB_ATTRIBUTE
@@ -292,9 +335,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
int malloc_dirname = 0;
glob_t dirs;
int retval = 0;
-#ifdef _LIBC
size_t alloca_used = 0;
-#endif
if (pattern == NULL || pglob == NULL || (flags & ~__GLOB_FLAGS) != 0)
{
@@ -308,7 +349,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
flags |= GLOB_ONLYDIR;
if (!(flags & GLOB_DOOFFS))
- /* Have to do this so `globfree' knows where to start freeing. It
+ /* Have to do this so 'globfree' knows where to start freeing. It
also makes all the code that uses gl_offs simpler. */
pglob->gl_offs = 0;
@@ -363,7 +404,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (begin != NULL)
{
/* Allocate working buffer large enough for our work. Note that
- we have at least an opening and closing brace. */
+ we have at least an opening and closing brace. */
size_t firstc;
char *alt_start;
const char *p;
@@ -372,16 +413,21 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
size_t rest_len;
char *onealt;
size_t pattern_len = strlen (pattern) - 1;
-#ifdef _LIBC
- int alloca_onealt = __libc_use_alloca (alloca_used + pattern_len);
+ int alloca_onealt = glob_use_alloca (alloca_used, pattern_len);
if (alloca_onealt)
onealt = alloca_account (pattern_len, alloca_used);
else
-#endif
{
- onealt = (char *) malloc (pattern_len);
+ onealt = malloc (pattern_len);
if (onealt == NULL)
- return GLOB_NOSPACE;
+ {
+ if (!(flags & GLOB_APPEND))
+ {
+ pglob->gl_pathc = 0;
+ pglob->gl_pathv = NULL;
+ }
+ return GLOB_NOSPACE;
+ }
}
/* We know the prefix for all sub-patterns. */
@@ -392,14 +438,11 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
next = next_brace_sub (begin + 1, flags);
if (next == NULL)
{
- /* It is an illegal expression. */
+ /* It is an invalid expression. */
illegal_brace:
-#ifdef _LIBC
if (__glibc_unlikely (!alloca_onealt))
-#endif
free (onealt);
- flags &= ~GLOB_BRACE;
- goto no_brace;
+ return glob (pattern, flags & ~GLOB_BRACE, errfunc, pglob);
}
/* Now find the end of the whole brace expression. */
@@ -437,9 +480,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
/* If we got an error, return it. */
if (result && result != GLOB_NOMATCH)
{
-#ifdef _LIBC
if (__glibc_unlikely (!alloca_onealt))
-#endif
free (onealt);
if (!(flags & GLOB_APPEND))
{
@@ -458,9 +499,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
assert (next != NULL);
}
-#ifdef _LIBC
if (__glibc_unlikely (!alloca_onealt))
-#endif
free (onealt);
if (pglob->gl_pathc != firstc)
@@ -471,7 +510,6 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
}
- no_brace:
oldcount = pglob->gl_pathc + pglob->gl_offs;
/* Find the filename. */
@@ -536,22 +574,20 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
char *drive_spec;
++dirlen;
- drive_spec = (char *) __alloca (dirlen + 1);
+ drive_spec = __alloca (dirlen + 1);
*((char *) mempcpy (drive_spec, pattern, dirlen)) = '\0';
/* For now, disallow wildcards in the drive spec, to
prevent infinite recursion in glob. */
if (__glob_pattern_p (drive_spec, !(flags & GLOB_NOESCAPE)))
return GLOB_NOMATCH;
- /* If this is "d:pattern", we need to copy `:' to DIRNAME
+ /* If this is "d:pattern", we need to copy ':' to DIRNAME
as well. If it's "d:/pattern", don't remove the slash
from "d:/", since "d:" and "d:/" are not the same.*/
}
#endif
-#ifdef _LIBC
- if (__libc_use_alloca (alloca_used + dirlen + 1))
+ if (glob_use_alloca (alloca_used, dirlen + 1))
newp = alloca_account (dirlen + 1, alloca_used);
else
-#endif
{
newp = malloc (dirlen + 1);
if (newp == NULL)
@@ -572,6 +608,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
/* "pattern/". Expand "pattern", appending slashes. */
{
int orig_flags = flags;
+ int val;
if (!(flags & GLOB_NOESCAPE) && dirname[dirlen - 1] == '\\')
{
/* "pattern\\/". Remove the final backslash if it hasn't
@@ -585,7 +622,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
flags &= ~(GLOB_NOCHECK | GLOB_NOMAGIC);
}
}
- int val = glob (dirname, flags | GLOB_MARK, errfunc, pglob);
+ val = glob (dirname, flags | GLOB_MARK, errfunc, pglob);
if (val == 0)
pglob->gl_flags = ((pglob->gl_flags & ~GLOB_MARK)
| (flags & GLOB_MARK));
@@ -602,7 +639,6 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
}
-#ifndef VMS
if ((flags & (GLOB_TILDE|GLOB_TILDE_CHECK)) && dirname[0] == '~')
{
if (dirname[1] == '\0' || dirname[1] == '/'
@@ -617,95 +653,129 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
home_dir = "SYS:";
# else
# ifdef WINDOWS32
+ /* Windows NT defines HOMEDRIVE and HOMEPATH. But give preference
+ to HOME, because the user can change HOME. */
if (home_dir == NULL || home_dir[0] == '\0')
- home_dir = "c:/users/default"; /* poor default */
+ {
+ const char *home_drive = getenv ("HOMEDRIVE");
+ const char *home_path = getenv ("HOMEPATH");
+
+ if (home_drive != NULL && home_path != NULL)
+ {
+ size_t home_drive_len = strlen (home_drive);
+ size_t home_path_len = strlen (home_path);
+ char *mem = alloca (home_drive_len + home_path_len + 1);
+
+ memcpy (mem, home_drive, home_drive_len);
+ memcpy (mem + home_drive_len, home_path, home_path_len + 1);
+ home_dir = mem;
+ }
+ else
+ home_dir = "c:/users/default"; /* poor default */
+ }
# else
if (home_dir == NULL || home_dir[0] == '\0')
{
int success;
char *name;
+ int malloc_name = 0;
size_t buflen = GET_LOGIN_NAME_MAX () + 1;
if (buflen == 0)
/* `sysconf' does not support _SC_LOGIN_NAME_MAX. Try
a moderate value. */
buflen = 20;
- name = alloca_account (buflen, alloca_used);
+ if (glob_use_alloca (alloca_used, buflen))
+ name = alloca_account (buflen, alloca_used);
+ else
+ {
+ name = malloc (buflen);
+ if (name == NULL)
+ {
+ retval = GLOB_NOSPACE;
+ goto out;
+ }
+ malloc_name = 1;
+ }
success = __getlogin_r (name, buflen) == 0;
if (success)
{
struct passwd *p;
-# if defined HAVE_GETPWNAM_R || defined _LIBC
- long int pwbuflen = GETPW_R_SIZE_MAX ();
+ char *malloc_pwtmpbuf = NULL;
char *pwtmpbuf;
+# if defined HAVE_GETPWNAM_R || defined _LIBC
+ long int pwbuflenmax = GETPW_R_SIZE_MAX ();
+ size_t pwbuflen = pwbuflenmax;
struct passwd pwbuf;
- int malloc_pwtmpbuf = 0;
int save = errno;
# ifndef _LIBC
- if (pwbuflen == -1)
+ if (! (0 < pwbuflenmax && pwbuflenmax <= SIZE_MAX))
/* `sysconf' does not support _SC_GETPW_R_SIZE_MAX.
Try a moderate value. */
pwbuflen = 1024;
# endif
- if (__libc_use_alloca (alloca_used + pwbuflen))
+ if (glob_use_alloca (alloca_used, pwbuflen))
pwtmpbuf = alloca_account (pwbuflen, alloca_used);
else
{
pwtmpbuf = malloc (pwbuflen);
if (pwtmpbuf == NULL)
{
+ if (__glibc_unlikely (malloc_name))
+ free (name);
retval = GLOB_NOSPACE;
goto out;
}
- malloc_pwtmpbuf = 1;
+ malloc_pwtmpbuf = pwtmpbuf;
}
while (getpwnam_r (name, &pwbuf, pwtmpbuf, pwbuflen, &p)
!= 0)
{
+ size_t newlen;
+ bool v;
if (errno != ERANGE)
{
p = NULL;
break;
}
-
- if (!malloc_pwtmpbuf
- && __libc_use_alloca (alloca_used
- + 2 * pwbuflen))
+ v = size_add_wrapv (pwbuflen, pwbuflen, &newlen);
+ if (!v && malloc_pwtmpbuf == NULL
+ && glob_use_alloca (alloca_used, newlen))
pwtmpbuf = extend_alloca_account (pwtmpbuf, pwbuflen,
- 2 * pwbuflen,
- alloca_used);
+ newlen, alloca_used);
else
{
- char *newp = realloc (malloc_pwtmpbuf
- ? pwtmpbuf : NULL,
- 2 * pwbuflen);
+ char *newp = (v ? NULL
+ : realloc (malloc_pwtmpbuf, newlen));
if (newp == NULL)
{
- if (__glibc_unlikely (malloc_pwtmpbuf))
- free (pwtmpbuf);
+ free (malloc_pwtmpbuf);
+ if (__glibc_unlikely (malloc_name))
+ free (name);
retval = GLOB_NOSPACE;
goto out;
}
- pwtmpbuf = newp;
- pwbuflen = 2 * pwbuflen;
- malloc_pwtmpbuf = 1;
+ malloc_pwtmpbuf = pwtmpbuf = newp;
}
+ pwbuflen = newlen;
__set_errno (save);
}
# else
p = getpwnam (name);
# endif
+ if (__glibc_unlikely (malloc_name))
+ free (name);
if (p != NULL)
{
- if (!malloc_pwtmpbuf)
+ if (malloc_pwtmpbuf == NULL)
home_dir = p->pw_dir;
else
{
size_t home_dir_len = strlen (p->pw_dir) + 1;
- if (__libc_use_alloca (alloca_used + home_dir_len))
+ if (glob_use_alloca (alloca_used, home_dir_len))
home_dir = alloca_account (home_dir_len,
alloca_used);
else
@@ -720,23 +790,30 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
malloc_home_dir = 1;
}
memcpy (home_dir, p->pw_dir, home_dir_len);
-
- free (pwtmpbuf);
}
}
+ free (malloc_pwtmpbuf);
+ }
+ else
+ {
+ if (__glibc_unlikely (malloc_name))
+ free (name);
}
}
if (home_dir == NULL || home_dir[0] == '\0')
{
+ if (__glibc_unlikely (malloc_home_dir))
+ free (home_dir);
if (flags & GLOB_TILDE_CHECK)
{
- if (__glibc_unlikely (malloc_home_dir))
- free (home_dir);
retval = GLOB_NOMATCH;
goto out;
}
else
- home_dir = (char *) "~"; /* No luck. */
+ {
+ home_dir = (char *) "~"; /* No luck. */
+ malloc_home_dir = 0;
+ }
}
# endif /* WINDOWS32 */
# endif
@@ -754,8 +831,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
{
char *newp;
size_t home_len = strlen (home_dir);
- int use_alloca = __libc_use_alloca (alloca_used
- + home_len + dirlen);
+ int use_alloca = glob_use_alloca (alloca_used, home_len + dirlen);
if (use_alloca)
newp = alloca_account (home_len + dirlen, alloca_used);
else
@@ -779,6 +855,9 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
dirname = newp;
dirlen += home_len - 1;
malloc_dirname = !use_alloca;
+
+ if (__glibc_unlikely (malloc_home_dir))
+ free (home_dir);
}
dirname_modified = 1;
}
@@ -806,7 +885,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
else
{
char *newp;
- if (__libc_use_alloca (alloca_used + (end_name - dirname)))
+ if (glob_use_alloca (alloca_used, end_name - dirname))
newp = alloca_account (end_name - dirname, alloca_used);
else
{
@@ -851,20 +930,21 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
/* Look up specific user's home directory. */
{
struct passwd *p;
+ char *malloc_pwtmpbuf = NULL;
# if defined HAVE_GETPWNAM_R || defined _LIBC
- long int buflen = GETPW_R_SIZE_MAX ();
+ long int buflenmax = GETPW_R_SIZE_MAX ();
+ size_t buflen = buflenmax;
char *pwtmpbuf;
- int malloc_pwtmpbuf = 0;
struct passwd pwbuf;
int save = errno;
# ifndef _LIBC
- if (buflen == -1)
- /* `sysconf' does not support _SC_GETPW_R_SIZE_MAX. Try a
+ if (! (0 <= buflenmax && buflenmax <= SIZE_MAX))
+ /* Perhaps 'sysconf' does not support _SC_GETPW_R_SIZE_MAX. Try a
moderate value. */
buflen = 1024;
# endif
- if (__libc_use_alloca (alloca_used + buflen))
+ if (glob_use_alloca (alloca_used, buflen))
pwtmpbuf = alloca_account (buflen, alloca_used);
else
{
@@ -877,32 +957,32 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
retval = GLOB_NOSPACE;
goto out;
}
- malloc_pwtmpbuf = 1;
+ malloc_pwtmpbuf = pwtmpbuf;
}
while (getpwnam_r (user_name, &pwbuf, pwtmpbuf, buflen, &p) != 0)
{
+ size_t newlen;
+ bool v;
if (errno != ERANGE)
{
p = NULL;
break;
}
- if (!malloc_pwtmpbuf
- && __libc_use_alloca (alloca_used + 2 * buflen))
+ v = size_add_wrapv (buflen, buflen, &newlen);
+ if (!v && malloc_pwtmpbuf == NULL
+ && glob_use_alloca (alloca_used, newlen))
pwtmpbuf = extend_alloca_account (pwtmpbuf, buflen,
- 2 * buflen, alloca_used);
+ newlen, alloca_used);
else
{
- char *newp = realloc (malloc_pwtmpbuf ? pwtmpbuf : NULL,
- 2 * buflen);
+ char *newp = v ? NULL : realloc (malloc_pwtmpbuf, newlen);
if (newp == NULL)
{
- if (__glibc_unlikely (malloc_pwtmpbuf))
- free (pwtmpbuf);
+ free (malloc_pwtmpbuf);
goto nomem_getpw;
}
- pwtmpbuf = newp;
- malloc_pwtmpbuf = 1;
+ malloc_pwtmpbuf = pwtmpbuf = newp;
}
__set_errno (save);
}
@@ -923,7 +1003,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
free (dirname);
malloc_dirname = 0;
- if (__libc_use_alloca (alloca_used + home_len + rest_len + 1))
+ if (glob_use_alloca (alloca_used, home_len + rest_len + 1))
dirname = alloca_account (home_len + rest_len + 1,
alloca_used);
else
@@ -931,8 +1011,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
dirname = malloc (home_len + rest_len + 1);
if (dirname == NULL)
{
- if (__glibc_unlikely (malloc_pwtmpbuf))
- free (pwtmpbuf);
+ free (malloc_pwtmpbuf);
retval = GLOB_NOSPACE;
goto out;
}
@@ -944,13 +1023,11 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
dirlen = home_len + rest_len;
dirname_modified = 1;
- if (__glibc_unlikely (malloc_pwtmpbuf))
- free (pwtmpbuf);
+ free (malloc_pwtmpbuf);
}
else
{
- if (__glibc_unlikely (malloc_pwtmpbuf))
- free (pwtmpbuf);
+ free (malloc_pwtmpbuf);
if (flags & GLOB_TILDE_CHECK)
/* We have to regard it as an error if we cannot find the
@@ -961,7 +1038,6 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
# endif /* Not Amiga && not WINDOWS32. */
}
-#endif /* Not VMS. */
/* Now test whether we looked for "~" or "~NAME". In this case we
can give the answer now. */
@@ -980,19 +1056,19 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
size_t newcount = pglob->gl_pathc + pglob->gl_offs;
char **new_gl_pathv;
- if (newcount > UINTPTR_MAX - (1 + 1)
- || newcount + 1 + 1 > ~((size_t) 0) / sizeof (char *))
+ if (newcount > SIZE_MAX / sizeof (char *) - 2)
{
nospace:
+ if (__glibc_unlikely (malloc_dirname))
+ free (dirname);
free (pglob->gl_pathv);
pglob->gl_pathv = NULL;
pglob->gl_pathc = 0;
return GLOB_NOSPACE;
}
- new_gl_pathv
- = (char **) realloc (pglob->gl_pathv,
- (newcount + 1 + 1) * sizeof (char *));
+ new_gl_pathv = realloc (pglob->gl_pathv,
+ (newcount + 2) * sizeof (char *));
if (new_gl_pathv == NULL)
goto nospace;
pglob->gl_pathv = new_gl_pathv;
@@ -1009,9 +1085,14 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
else
{
- pglob->gl_pathv[newcount] = strdup (dirname);
- if (pglob->gl_pathv[newcount] == NULL)
- goto nospace;
+ if (__glibc_unlikely (malloc_dirname))
+ pglob->gl_pathv[newcount] = dirname;
+ else
+ {
+ pglob->gl_pathv[newcount] = strdup (dirname);
+ if (pglob->gl_pathv[newcount] == NULL)
+ goto nospace;
+ }
}
pglob->gl_pathv[++newcount] = NULL;
++pglob->gl_pathc;
@@ -1078,7 +1159,7 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
{
size_t old_pathc;
-#ifdef SHELL
+#ifdef SHELL
{
/* Make globbing interruptible in the bash shell. */
extern int interrupt_state;
@@ -1134,22 +1215,20 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
size_t newcount = pglob->gl_pathc + pglob->gl_offs;
char **new_gl_pathv;
- if (newcount > UINTPTR_MAX - 2
- || newcount + 2 > ~((size_t) 0) / sizeof (char *))
+ if (newcount > SIZE_MAX / sizeof (char *) - 2)
{
nospace2:
globfree (&dirs);
return GLOB_NOSPACE;
}
- new_gl_pathv = (char **) realloc (pglob->gl_pathv,
- (newcount + 2)
- * sizeof (char *));
+ new_gl_pathv = realloc (pglob->gl_pathv,
+ (newcount + 2) * sizeof (char *));
if (new_gl_pathv == NULL)
goto nospace2;
pglob->gl_pathv = new_gl_pathv;
- pglob->gl_pathv[newcount] = __strdup (pattern);
+ pglob->gl_pathv[newcount] = strdup (pattern);
if (pglob->gl_pathv[newcount] == NULL)
{
globfree (&dirs);
@@ -1276,7 +1355,7 @@ libc_hidden_def (glob)
#endif
-#if !defined _LIBC || !defined GLOB_ONLY_P
+#ifndef GLOB_ONLY_P
/* Free storage allocated in PGLOB by a previous `glob' call. */
void
@@ -1300,8 +1379,8 @@ libc_hidden_def (globfree)
static int
collated_compare (const void *a, const void *b)
{
- const char *const s1 = *(const char *const * const) a;
- const char *const s2 = *(const char *const * const) b;
+ char *const *ps1 = a; char *s1 = *ps1;
+ char *const *ps2 = b; char *s2 = *ps2;
if (s1 == s2)
return 0;
@@ -1351,7 +1430,7 @@ prefix_array (const char *dirname, char **array, size_t n)
for (i = 0; i < n; ++i)
{
size_t eltlen = strlen (array[i]) + 1;
- char *new = (char *) malloc (dirlen + 1 + eltlen);
+ char *new = malloc (dirlen + 1 + eltlen);
if (new == NULL)
{
while (i > 0)
@@ -1373,7 +1452,7 @@ prefix_array (const char *dirname, char **array, size_t n)
/* We must not compile this function twice. */
-#if !defined _LIBC || !defined NO_GLOB_PATTERN_P
+#ifndef NO_GLOB_PATTERN_P
int
__glob_pattern_type (const char *pattern, int quote)
{
@@ -1421,50 +1500,55 @@ weak_alias (__glob_pattern_p, glob_pattern_p)
# endif
#endif
-#endif /* !GLOB_ONLY_P */
-
/* We put this in a separate function mainly to allow the memory
allocated with alloca to be recycled. */
-#if !defined _LIBC || !defined GLOB_ONLY_P
static int
__attribute_noinline__
link_exists2_p (const char *dir, size_t dirlen, const char *fname,
glob_t *pglob
-# ifndef _LIBC
+# if !defined _LIBC && !HAVE_FSTATAT
, int flags
# endif
)
{
size_t fnamelen = strlen (fname);
- char *fullname = (char *) __alloca (dirlen + 1 + fnamelen + 1);
+ char *fullname = __alloca (dirlen + 1 + fnamelen + 1);
struct stat st;
-# ifndef _LIBC
- struct_stat64 st64;
-# endif
mempcpy (mempcpy (mempcpy (fullname, dir, dirlen), "/", 1),
fname, fnamelen + 1);
-# ifdef _LIBC
- return (*pglob->gl_stat) (fullname, &st) == 0;
-# else
- return ((__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
- ? (*pglob->gl_stat) (fullname, &st)
- : __stat64 (fullname, &st64)) == 0);
+# if !defined _LIBC && !HAVE_FSTATAT
+ if (__builtin_expect ((flags & GLOB_ALTDIRFUNC) == 0, 1))
+ {
+ struct_stat64 st64;
+ return __stat64 (fullname, &st64) == 0;
+ }
# endif
+ return (*pglob->gl_stat) (fullname, &st) == 0;
}
-# ifdef _LIBC
-# define link_exists_p(dfd, dirname, dirnamelen, fname, pglob, flags) \
- (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0) \
- ? link_exists2_p (dirname, dirnamelen, fname, pglob) \
- : ({ struct stat64 st64; \
- __fxstatat64 (_STAT_VER, dfd, fname, &st64, 0) == 0; }))
+
+/* Return true if DIR/FNAME exists. */
+static int
+link_exists_p (int dfd, const char *dir, size_t dirlen, const char *fname,
+ glob_t *pglob, int flags)
+{
+# if defined _LIBC || HAVE_FSTATAT
+ if (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0))
+ return link_exists2_p (dir, dirlen, fname, pglob);
+ else
+ {
+ /* dfd cannot be -1 here, because dirfd never returns -1 on
+ glibc, or on hosts that have fstatat. */
+ struct_stat64 st64;
+ return __fxstatat64 (_STAT_VER, dfd, fname, &st64, 0) == 0;
+ }
# else
-# define link_exists_p(dfd, dirname, dirnamelen, fname, pglob, flags) \
- link_exists2_p (dirname, dirnamelen, fname, pglob, flags)
+ return link_exists2_p (dir, dirlen, fname, pglob, flags);
# endif
-#endif
+}
+#endif /* !defined GLOB_ONLY_P */
/* Like `glob', but PATTERN is a final pathname component,
@@ -1492,6 +1576,7 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
size_t cur = 0;
int meta;
int save;
+ int result;
alloca_used += sizeof (init_names);
@@ -1516,14 +1601,16 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
struct_stat64 st64;
} ust;
size_t patlen = strlen (pattern);
- int alloca_fullname = __libc_use_alloca (alloca_used
- + dirlen + 1 + patlen + 1);
+ size_t fullsize;
+ bool alloca_fullname
+ = (! size_add_wrapv (dirlen + 1, patlen + 1, &fullsize)
+ && glob_use_alloca (alloca_used, fullsize));
char *fullname;
if (alloca_fullname)
- fullname = alloca_account (dirlen + 1 + patlen + 1, alloca_used);
+ fullname = alloca_account (fullsize, alloca_used);
else
{
- fullname = malloc (dirlen + 1 + patlen + 1);
+ fullname = malloc (fullsize);
if (fullname == NULL)
return GLOB_NOSPACE;
}
@@ -1555,10 +1642,8 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
}
else
{
-#ifdef _LIBC
int dfd = (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
? -1 : dirfd ((DIR *) stream));
-#endif
int fnm_flags = ((!(flags & GLOB_PERIOD) ? FNM_PERIOD : 0)
| ((flags & GLOB_NOESCAPE) ? FNM_NOESCAPE : 0)
#if defined _AMIGA || defined VMS
@@ -1607,7 +1692,7 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
size_t size = (sizeof (struct globnames)
+ ((count - INITIAL_COUNT)
* sizeof (char *)));
- if (__libc_use_alloca (alloca_used + size))
+ if (glob_use_alloca (alloca_used, size))
newnames = names_alloca
= alloca_account (size, alloca_used);
else if ((newnames = malloc (size))
@@ -1623,6 +1708,8 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
goto memory_error;
++cur;
++nfound;
+ if (SIZE_MAX - pglob->gl_offs <= nfound)
+ goto memory_error;
}
}
}
@@ -1633,36 +1720,35 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
{
size_t len = strlen (pattern);
nfound = 1;
- names->name[cur] = (char *) malloc (len + 1);
+ names->name[cur] = malloc (len + 1);
if (names->name[cur] == NULL)
goto memory_error;
*((char *) mempcpy (names->name[cur++], pattern, len)) = '\0';
}
- int result = GLOB_NOMATCH;
+ result = GLOB_NOMATCH;
if (nfound != 0)
{
+ char **new_gl_pathv;
result = 0;
- if (pglob->gl_pathc > UINTPTR_MAX - pglob->gl_offs
- || pglob->gl_pathc + pglob->gl_offs > UINTPTR_MAX - nfound
- || pglob->gl_pathc + pglob->gl_offs + nfound > UINTPTR_MAX - 1
- || (pglob->gl_pathc + pglob->gl_offs + nfound + 1
- > UINTPTR_MAX / sizeof (char *)))
+ if (SIZE_MAX / sizeof (char *) - pglob->gl_pathc
+ < pglob->gl_offs + nfound + 1)
goto memory_error;
- char **new_gl_pathv;
new_gl_pathv
- = (char **) realloc (pglob->gl_pathv,
- (pglob->gl_pathc + pglob->gl_offs + nfound + 1)
- * sizeof (char *));
+ = realloc (pglob->gl_pathv,
+ (pglob->gl_pathc + pglob->gl_offs + nfound + 1)
+ * sizeof (char *));
+
if (new_gl_pathv == NULL)
{
memory_error:
while (1)
{
struct globnames *old = names;
- for (size_t i = 0; i < cur; ++i)
+ size_t i;
+ for (i = 0; i < cur; ++i)
free (names->name[i]);
names = names->next;
/* NB: we will not leak memory here if we exit without
@@ -1687,7 +1773,8 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
while (1)
{
struct globnames *old = names;
- for (size_t i = 0; i < cur; ++i)
+ size_t i;
+ for (i = 0; i < cur; ++i)
new_gl_pathv[pglob->gl_offs + pglob->gl_pathc++]
= names->name[i];
names = names->next;
diff --git a/posix/globtest.c b/posix/globtest.c
index 878ae33..7ffcb91 100644
--- a/posix/globtest.c
+++ b/posix/globtest.c
@@ -114,5 +114,8 @@ main (int argc, char *argv[])
g.gl_pathv[i] ? g.gl_pathv[i] : "(null)",
quotes ? "'" : "");
}
+
+ globfree (&g);
+
return 0;
}
diff --git a/sysdeps/unix/sysv/linux/i386/glob64.c b/sysdeps/unix/sysv/linux/i386/glob64.c
index f681951..9c7abd8 100644
--- a/sysdeps/unix/sysv/linux/i386/glob64.c
+++ b/sysdeps/unix/sysv/linux/i386/glob64.c
@@ -70,6 +70,8 @@ int __old_glob64 (const char *__pattern, int __flags,
#define GLOB_ONLY_P 1
+#define GLOB_COMPAT_BUILD 1
+
#include <posix/glob.c>
compat_symbol (libc, __old_glob64, glob64, GLIBC_2_1);
--
2.7.4
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 03/17] support: Add optstring support
2017-06-08 21:13 ` [PATCH 03/17] support: Add optstring support Adhemerval Zanella
@ 2017-06-13 9:22 ` Florian Weimer
2017-06-13 11:55 ` Adhemerval Zanella
0 siblings, 1 reply; 31+ messages in thread
From: Florian Weimer @ 2017-06-13 9:22 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
On 06/08/2017 11:13 PM, Adhemerval Zanella wrote:
> +#ifdef CMDLINE_OPTSTRING
> + test_config.optstring = CMDLINE_OPTSTRING;
> +#else
> + test_config.optstring = "+";
Why not "+" CMDLINE_OPTSTRING, for consistency?
Otherwise, this looks good to me.
Thanks,
Florian
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/17] malloc: Add specialized dynarray for C strings
2017-06-08 21:14 ` [PATCH 07/17] malloc: Add specialized dynarray for C strings Adhemerval Zanella
@ 2017-06-13 9:46 ` Florian Weimer
2017-06-13 12:17 ` Adhemerval Zanella
0 siblings, 1 reply; 31+ messages in thread
From: Florian Weimer @ 2017-06-13 9:46 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> 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.
It's surprising that glob needs these many functions.
I'm not sure if the automatic NUL termination makes sense if we ever
want to generalize this interface. How ingrained is that to the glob
use case?
> +++ b/malloc/char_array.c
Should this be malloc/char_array-skeleton.c, to indicate that this file
is parameterized and intended for inclusion into another .c file?
> +#define DYNARRAY_ELEMENT_INIT(__e) (*__e = '\0')
Why is this needed? I don't think the code cares for the contents of
freshly added bytes. For the trailing NUL byte, it seems better to add
that separately from the initialization operations.
> +static bool __attribute_used__
> +char_array_replace_str_pos (struct char_array *array, size_t pos,
> + const char *str, size_t len)
> +{
> + if (pos > array->dynarray_header.used)
> + return false;
> +
> + size_t newsize;
> + if (check_add_wrapv_size_t (pos, len, &newsize)
> + || check_add_wrapv_size_t (newsize, 1, &newsize)
> + || !char_array_resize (array, newsize))
> + return false;
I don't think it is a good idea to mix the reporting of usage errors
(index out of bounds) with environmental conditions (memory allocation
failure). Have you considered calling __libc_fatal if pos is out of
range, similar to the existing *_at function?
> +++ b/malloc/malloc-internal.h
> +/* 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_wrapv_size_t (size_t a, size_t b, size_t *r)
Why not use check_add_overflow_size_t, for consistency with
check_mul_overflow_size_t?
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 03/17] support: Add optstring support
2017-06-13 9:22 ` Florian Weimer
@ 2017-06-13 11:55 ` Adhemerval Zanella
0 siblings, 0 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-13 11:55 UTC (permalink / raw)
To: Florian Weimer, libc-alpha
On 13/06/2017 06:22, Florian Weimer wrote:
> On 06/08/2017 11:13 PM, Adhemerval Zanella wrote:
>> +#ifdef CMDLINE_OPTSTRING
>> + test_config.optstring = CMDLINE_OPTSTRING;
>> +#else
>> + test_config.optstring = "+";
>
> Why not "+" CMDLINE_OPTSTRING, for consistency?
>
> Otherwise, this looks good to me.
>
> Thanks,
> Florian
>
I will add this change.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/17] malloc: Add specialized dynarray for C strings
2017-06-13 9:46 ` Florian Weimer
@ 2017-06-13 12:17 ` Adhemerval Zanella
2017-06-13 14:24 ` Adhemerval Zanella
2017-06-14 13:35 ` Florian Weimer
0 siblings, 2 replies; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-13 12:17 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 13/06/2017 06:46, Florian Weimer wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>
>> 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.
>
> It's surprising that glob needs these many functions.
>
> I'm not sure if the automatic NUL termination makes sense if we ever
> want to generalize this interface. How ingrained is that to the glob
> use case?
glob internally prepends the search directory by replacing the tilde with
a external home directory (GLOB_TILDE, either by environment variable or
through getXXX functions) or append new data by calling new patterns
recursively (GLOB_BRACE).
It is not an specific usage to glob, but rather a generic one which with
more higher level languages all the memory buffer bikeshedding will be
avoided by a string object. The idea is to keep a consistent C string buffer
stack-allocated (for general use) or heap based (to not bound due the
internal expansion) which can be used as constant argument for other
function call (fnmatch or recursively in case of GLOB_BRACE).
>
>> +++ b/malloc/char_array.c
>
> Should this be malloc/char_array-skeleton.c, to indicate that this file
> is parameterized and intended for inclusion into another .c file?
I am not sure, the idea of char_array is to be self-contained, there is
no need to define anything in the file to include it. Also, if we see
other usage of dynamic C string inside glibc it would be a good idea to
add internal symbols for common symbols.
>
>> +#define DYNARRAY_ELEMENT_INIT(__e) (*__e = '\0')
>
> Why is this needed? I don't think the code cares for the contents of
> freshly added bytes. For the trailing NUL byte, it seems better to add
> that separately from the initialization operations.
It does not, it is an artefact I added and it should be removed.
>
>> +static bool __attribute_used__
>> +char_array_replace_str_pos (struct char_array *array, size_t pos,
>> + const char *str, size_t len)
>> +{
>> + if (pos > array->dynarray_header.used)
>> + return false;
>> +
>> + size_t newsize;
>> + if (check_add_wrapv_size_t (pos, len, &newsize)
>> + || check_add_wrapv_size_t (newsize, 1, &newsize)
>> + || !char_array_resize (array, newsize))
>> + return false;
>
> I don't think it is a good idea to mix the reporting of usage errors
> (index out of bounds) with environmental conditions (memory allocation
> failure). Have you considered calling __libc_fatal if pos is out of
> range, similar to the existing *_at function?
Indeed, I will change to call __libc_fatal in such cases.
>
>> +++ b/malloc/malloc-internal.h
>
>> +/* 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_wrapv_size_t (size_t a, size_t b, size_t *r)
>
> Why not use check_add_overflow_size_t, for consistency with
> check_mul_overflow_size_t?
>
I will change it.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/17] malloc: Add specialized dynarray for C strings
2017-06-13 12:17 ` Adhemerval Zanella
@ 2017-06-13 14:24 ` Adhemerval Zanella
2017-06-13 20:51 ` Adhemerval Zanella
2017-06-14 13:35 ` Florian Weimer
1 sibling, 1 reply; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-13 14:24 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 13/06/2017 09:17, Adhemerval Zanella wrote:
>
>
> On 13/06/2017 06:46, Florian Weimer wrote:
>> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>>
>>> 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.
>>
>> It's surprising that glob needs these many functions.
>>
>> I'm not sure if the automatic NUL termination makes sense if we ever
>> want to generalize this interface. How ingrained is that to the glob
>> use case?
>
> glob internally prepends the search directory by replacing the tilde with
> a external home directory (GLOB_TILDE, either by environment variable or
> through getXXX functions) or append new data by calling new patterns
> recursively (GLOB_BRACE).
>
> It is not an specific usage to glob, but rather a generic one which with
> more higher level languages all the memory buffer bikeshedding will be
> avoided by a string object. The idea is to keep a consistent C string buffer
> stack-allocated (for general use) or heap based (to not bound due the
> internal expansion) which can be used as constant argument for other
> function call (fnmatch or recursively in case of GLOB_BRACE).
>
>>
>>> +++ b/malloc/char_array.c
>>
>> Should this be malloc/char_array-skeleton.c, to indicate that this file
>> is parameterized and intended for inclusion into another .c file?
>
> I am not sure, the idea of char_array is to be self-contained, there is
> no need to define anything in the file to include it. Also, if we see
> other usage of dynamic C string inside glibc it would be a good idea to
> add internal symbols for common symbols.
>
>>
>>> +#define DYNARRAY_ELEMENT_INIT(__e) (*__e = '\0')
>>
>> Why is this needed? I don't think the code cares for the contents of
>> freshly added bytes. For the trailing NUL byte, it seems better to add
>> that separately from the initialization operations.
>
> It does not, it is an artefact I added and it should be removed.
>
>>
>>> +static bool __attribute_used__
>>> +char_array_replace_str_pos (struct char_array *array, size_t pos,
>>> + const char *str, size_t len)
>>> +{
>>> + if (pos > array->dynarray_header.used)
>>> + return false;
>>> +
>>> + size_t newsize;
>>> + if (check_add_wrapv_size_t (pos, len, &newsize)
>>> + || check_add_wrapv_size_t (newsize, 1, &newsize)
>>> + || !char_array_resize (array, newsize))
>>> + return false;
>>
>> I don't think it is a good idea to mix the reporting of usage errors
>> (index out of bounds) with environmental conditions (memory allocation
>> failure). Have you considered calling __libc_fatal if pos is out of
>> range, similar to the existing *_at function?
>
> Indeed, I will change to call __libc_fatal in such cases.
>
>>
>>> +++ b/malloc/malloc-internal.h
>>
>>> +/* 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_wrapv_size_t (size_t a, size_t b, size_t *r)
>>
>> Why not use check_add_overflow_size_t, for consistency with
>> check_mul_overflow_size_t?
>>
>
> I will change it.
>
Here is an updated patch for the specialized dynarray. I have incorporated all
your suggestion but the skeleton name change (which I am not sure if it should
follow the idea since it should be a complete 'api'). I also added some nonull
attribute as for dynarray implementation.
----
diff --git a/malloc/Makefile b/malloc/Makefile
index 14c13f1..323b3fb 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -46,6 +46,7 @@ tests-internal += \
tst-dynarray \
tst-dynarray-fail \
tst-dynarray-at-fail \
+ tst-char_array
ifneq (no,$(have-tunables))
tests += tst-malloc-usable-tunables
@@ -58,7 +59,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 \
diff --git a/malloc/char_array.c b/malloc/char_array.c
new file mode 100644
index 0000000..4d73203
--- /dev/null
+++ b/malloc/char_array.c
@@ -0,0 +1,281 @@
+/* Specialized dynarray for C strings.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+/* This file provides a dynamic C string with an initial stack allocated
+ buffer. Since it is based on dynarray, it provided 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_resize_str (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_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 = "testi";
+ char_array_erase (&str, 5);
+ // str = "123testi";
+ char_array_prepend_str (&str, "123");
+ // len = 8;
+ size_t len = char_array_length (&str);
+ // str = "123testi456";
+ char_array_append_str (&str, "456");
+ // str = "123testi789";
+ 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>
+
+/* Return a const char for the internal C string handled by 'array'. */
+__attribute__ ((nonnull (1)))
+static const char *
+char_array_str (struct char_array *array)
+{
+ return char_array_at (array, 0);
+}
+
+/* Return the character at position 'pos' from the char_array 'array'. */
+__attribute__ ((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 (struct char_array *array)
+{
+ /* Exclude the final '\0'. */
+ return array->dynarray_header.used - 1;
+}
+
+/* Copy the contents of string 'str' to char_array 'array', including the
+ final '\0'. */
+__attribute__ ((nonnull (1, 2)))
+static bool
+char_array_set_str (struct char_array *array, const char *str)
+{
+ size_t size = strlen (str) + 1;
+ if (!char_array_resize (array, size))
+ return false;
+ memcpy (array->dynarray_header.array, str, size);
+ array->dynarray_header.used = size;
+ return true;
+}
+
+/* Copy up 'size' bytes from string 'str' to char_array 'array'. A final
+ '\0' is appended in the char_array. */
+__attribute__ ((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_wrapv_size_t (size, 1, &newsize))
+ __libc_dynarray_overflow_failure (size, 1);
+
+ if (!char_array_resize (array, newsize))
+ return false;
+
+ *((char *) mempcpy (array->dynarray_header.array, str, size)) = '\0';
+ array->dynarray_header.used = newsize;
+ return true;
+}
+
+/* Initialize the char_array 'array' and sets it to an empty string (""). */
+__attribute__ ((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__ ((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__ ((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__ ((nonnull (1)))
+static bool
+char_array_is_empty (struct char_array *array)
+{
+ return *char_array_at (array, 0) == '\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__ ((nonnull (1)))
+static bool
+char_array_erase (struct char_array *array, size_t pos)
+{
+ if (pos >= array->dynarray_header.used - 1)
+ return false;
+
+ char *ppos = char_array_at (array, pos);
+ char *lpos = array->dynarray_header.array + array->dynarray_header.used;
+ ptrdiff_t size = lpos - ppos;
+ memmove (ppos, ppos + 1, size);
+ array->dynarray_header.used--;
+ return true;
+}
+
+/* Resize the char_array 'array' to size 'count' maintaining the ending
+ '\0' byte. */
+__attribute__ ((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[array->dynarray_header.used] = '\0';
+ return true;
+}
+
+/* Prepend the contents of string 'str' to char_array 'array', including the
+ final '\0' byte. */
+__attribute__ ((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_wrapv_size_t (used, size, &newsize))
+ __libc_dynarray_overflow_failure (used, size);
+
+ if (!char_array_resize (array, newsize))
+ return false;
+
+ /* Make room for the string and copy it. */
+ memmove (array->dynarray_header.array + size, array->dynarray_header.array,
+ used);
+ memcpy (array->dynarray_header.array, str, size);
+ array->dynarray_header.used = newsize;
+ return true;
+}
+
+/* Append the contents of string 'str' to char_array 'array, including the
+ final '\0' byte. */
+__attribute__ ((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_wrapv_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';
+ array->dynarray_header.used = newsize;
+ return true;
+}
+
+/* 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__ ((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)
+ return false;
+
+ size_t newsize;
+ if (check_add_wrapv_size_t (pos, len, &newsize))
+ __libc_dynarray_overflow_failure (pos, len);
+ if (check_add_wrapv_size_t (newsize, 1, &newsize))
+ __libc_dynarray_overflow_failure (newsize, 1);
+
+ if (!char_array_resize (array, newsize))
+ return false;
+
+ char *start = char_array_at (array, pos);
+ *(char *) mempcpy (start, str, len) = '\0';
+ array->dynarray_header.used = newsize;
+ return true;
+}
diff --git a/malloc/dynarray.h b/malloc/dynarray.h
index c73e08b..5a491b4 100644
--- a/malloc/dynarray.h
+++ b/malloc/dynarray.h
@@ -173,4 +173,12 @@ void __libc_dynarray_at_failure (size_t size, size_t index)
__attribute__ ((noreturn));
libc_hidden_proto (__libc_dynarray_at_failure)
+/* 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));
+libc_hidden_proto (__libc_dynarray_overflow_failure)
+
#endif /* _DYNARRAY_H */
diff --git a/malloc/dynarray_overflow_failure.c b/malloc/dynarray_overflow_failure.c
new file mode 100644
index 0000000..14936b0
--- /dev/null
+++ b/malloc/dynarray_overflow_failure.c
@@ -0,0 +1,31 @@
+/* Report an dynamic array size overflow condition.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <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 dbd801a..3066cd3 100644
--- a/malloc/malloc-internal.h
+++ b/malloc/malloc-internal.h
@@ -101,4 +101,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_wrapv_size_t (size_t a, size_t b, size_t *r)
+{
+#if 5 <= __GNUC__
+ 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..53f9482
--- /dev/null
+++ b/malloc/tst-char_array.c
@@ -0,0 +1,107 @@
+/* Test for char_array.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <string.h>
+
+#include <malloc/char_array.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"));
+ char_array_free (&str);
+ }
+
+ return 0;
+}
+
+#include <support/test-driver.c>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/17] malloc: Add specialized dynarray for C strings
2017-06-13 14:24 ` Adhemerval Zanella
@ 2017-06-13 20:51 ` Adhemerval Zanella
2017-06-14 14:05 ` Florian Weimer
0 siblings, 1 reply; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-13 20:51 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 13/06/2017 11:24, Adhemerval Zanella wrote:
> Here is an updated patch for the specialized dynarray. I have incorporated all
> your suggestion but the skeleton name change (which I am not sure if it should
> follow the idea since it should be a complete 'api'). I also added some nonull
> attribute as for dynarray implementation.
I rebase against the new begin/end additions and fixed some issues regarding
the make check (which I forgot to actually run before patch submission...).
---
diff --git a/malloc/Makefile b/malloc/Makefile
index 14c13f1..323b3fb 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -46,6 +46,7 @@ tests-internal += \
tst-dynarray \
tst-dynarray-fail \
tst-dynarray-at-fail \
+ tst-char_array
ifneq (no,$(have-tunables))
tests += tst-malloc-usable-tunables
@@ -58,7 +59,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 \
diff --git a/malloc/Versions b/malloc/Versions
index 5b54306..86b52f7 100644
--- a/malloc/Versions
+++ b/malloc/Versions
@@ -82,6 +82,7 @@ libc {
# dynarray support
__libc_dynarray_at_failure;
+ __libc_dynarray_overflow_failure;
__libc_dynarray_emplace_enlarge;
__libc_dynarray_finalize;
__libc_dynarray_resize;
diff --git a/malloc/char_array.c b/malloc/char_array.c
new file mode 100644
index 0000000..e37df81
--- /dev/null
+++ b/malloc/char_array.c
@@ -0,0 +1,281 @@
+/* Specialized dynarray for C strings.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+/* This file provides a dynamic C string with an initial stack allocated
+ buffer. Since it is based on dynarray, it provided 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_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 = "testi";
+ char_array_erase (&str, 5);
+ // str = "123testi";
+ char_array_prepend_str (&str, "123");
+ // len = 8;
+ size_t len = char_array_length (&str);
+ // str = "123testi456";
+ char_array_append_str (&str, "456");
+ // str = "123testi789";
+ 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>
+
+/* Return a const char for the internal C string handled by 'array'. */
+__attribute__ ((nonnull (1)))
+static const char *
+char_array_str (struct char_array *array)
+{
+ return char_array_at (array, 0);
+}
+
+/* Return the character at position 'pos' from the char_array 'array'. */
+__attribute__ ((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 (struct char_array *array)
+{
+ /* Exclude the final '\0'. */
+ return array->dynarray_header.used - 1;
+}
+
+/* Copy the contents of string 'str' to char_array 'array', including the
+ final '\0'. */
+__attribute__ ((nonnull (1, 2)))
+static bool
+char_array_set_str (struct char_array *array, const char *str)
+{
+ size_t size = strlen (str) + 1;
+ if (!char_array_resize (array, size))
+ return false;
+ memcpy (array->dynarray_header.array, str, size);
+ array->dynarray_header.used = size;
+ return true;
+}
+
+/* Copy up 'size' bytes from string 'str' to char_array 'array'. A final
+ '\0' is appended in the char_array. */
+__attribute__ ((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_wrapv_size_t (size, 1, &newsize))
+ __libc_dynarray_overflow_failure (size, 1);
+
+ if (!char_array_resize (array, newsize))
+ return false;
+
+ *((char *) mempcpy (array->dynarray_header.array, str, size)) = '\0';
+ array->dynarray_header.used = newsize;
+ return true;
+}
+
+/* Initialize the char_array 'array' and sets it to an empty string (""). */
+__attribute__ ((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__ ((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__ ((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__ ((nonnull (1)))
+static bool
+char_array_is_empty (struct char_array *array)
+{
+ return *char_array_at (array, 0) == '\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__ ((nonnull (1)))
+static bool
+char_array_erase (struct char_array *array, size_t pos)
+{
+ if (pos >= array->dynarray_header.used - 1)
+ return false;
+
+ char *ppos = char_array_at (array, pos);
+ char *lpos = array->dynarray_header.array + array->dynarray_header.used;
+ ptrdiff_t size = lpos - ppos;
+ memmove (ppos, ppos + 1, size);
+ array->dynarray_header.used--;
+ return true;
+}
+
+/* Resize the char_array 'array' to size 'count' maintaining the ending
+ '\0' byte. */
+__attribute__ ((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__ ((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_wrapv_size_t (used, size, &newsize))
+ __libc_dynarray_overflow_failure (used, size);
+
+ if (!char_array_resize (array, newsize))
+ return false;
+
+ /* Make room for the string and copy it. */
+ memmove (array->dynarray_header.array + size, array->dynarray_header.array,
+ used);
+ memcpy (array->dynarray_header.array, str, size);
+ array->dynarray_header.used = newsize;
+ return true;
+}
+
+/* Append the contents of string 'str' to char_array 'array, including the
+ final '\0' byte. */
+__attribute__ ((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_wrapv_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';
+ array->dynarray_header.used = newsize;
+ return true;
+}
+
+/* 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__ ((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)
+ return false;
+
+ size_t newsize;
+ if (check_add_wrapv_size_t (pos, len, &newsize))
+ __libc_dynarray_overflow_failure (pos, len);
+ if (check_add_wrapv_size_t (newsize, 1, &newsize))
+ __libc_dynarray_overflow_failure (newsize, 1);
+
+ if (!char_array_resize (array, newsize))
+ return false;
+
+ char *start = char_array_at (array, pos);
+ *(char *) mempcpy (start, str, len) = '\0';
+ array->dynarray_header.used = newsize;
+ return true;
+}
diff --git a/malloc/dynarray.h b/malloc/dynarray.h
index c73e08b..5a491b4 100644
--- a/malloc/dynarray.h
+++ b/malloc/dynarray.h
@@ -173,4 +173,12 @@ void __libc_dynarray_at_failure (size_t size, size_t index)
__attribute__ ((noreturn));
libc_hidden_proto (__libc_dynarray_at_failure)
+/* 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));
+libc_hidden_proto (__libc_dynarray_overflow_failure)
+
#endif /* _DYNARRAY_H */
diff --git a/malloc/dynarray_overflow_failure.c b/malloc/dynarray_overflow_failure.c
new file mode 100644
index 0000000..14936b0
--- /dev/null
+++ b/malloc/dynarray_overflow_failure.c
@@ -0,0 +1,31 @@
+/* Report an dynamic array size overflow condition.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <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 dbd801a..3066cd3 100644
--- a/malloc/malloc-internal.h
+++ b/malloc/malloc-internal.h
@@ -101,4 +101,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_wrapv_size_t (size_t a, size_t b, size_t *r)
+{
+#if 5 <= __GNUC__
+ 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..cb06b9c
--- /dev/null
+++ b/malloc/tst-char_array.c
@@ -0,0 +1,110 @@
+/* Test for char_array.
+ Copyright (C) 2017 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#include <string.h>
+
+#include <malloc/char_array.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);
+ char_array_free (&str);
+ }
+
+ return 0;
+}
+
+#include <support/test-driver.c>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/17] malloc: Add specialized dynarray for C strings
2017-06-13 12:17 ` Adhemerval Zanella
2017-06-13 14:24 ` Adhemerval Zanella
@ 2017-06-14 13:35 ` Florian Weimer
2017-06-14 14:04 ` Adhemerval Zanella
1 sibling, 1 reply; 31+ messages in thread
From: Florian Weimer @ 2017-06-14 13:35 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>>> +++ b/malloc/char_array.c
>>
>> Should this be malloc/char_array-skeleton.c, to indicate that this file
>> is parameterized and intended for inclusion into another .c file?
>
> I am not sure, the idea of char_array is to be self-contained, there is
> no need to define anything in the file to include it. Also, if we see
> other usage of dynamic C string inside glibc it would be a good idea to
> add internal symbols for common symbols.
But it's still necessary to #include the file because it is what the C++
people call a header-only library. And there is a hook for customizing
the size of the stack allocation. This is why I asked. I don't have a
strong opinion on file naming, though.
Florian
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/17] malloc: Add specialized dynarray for C strings
2017-06-14 13:35 ` Florian Weimer
@ 2017-06-14 14:04 ` Adhemerval Zanella
2017-06-14 14:06 ` Florian Weimer
0 siblings, 1 reply; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-14 14:04 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 14/06/2017 10:35, Florian Weimer wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>
>>>> +++ b/malloc/char_array.c
>>>
>>> Should this be malloc/char_array-skeleton.c, to indicate that this file
>>> is parameterized and intended for inclusion into another .c file?
>>
>> I am not sure, the idea of char_array is to be self-contained, there is
>> no need to define anything in the file to include it. Also, if we see
>> other usage of dynamic C string inside glibc it would be a good idea to
>> add internal symbols for common symbols.
>
> But it's still necessary to #include the file because it is what the C++
> people call a header-only library. And there is a hook for customizing
> the size of the stack allocation. This is why I asked. I don't have a
> strong opinion on file naming, though.
Fair enough then, although if this kind of usage pattern for dynarray
become common it would be interesting to move to internal symbol. I will
change to char_array-skeleton.c.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/17] malloc: Add specialized dynarray for C strings
2017-06-13 20:51 ` Adhemerval Zanella
@ 2017-06-14 14:05 ` Florian Weimer
2017-06-14 18:43 ` Adhemerval Zanella
0 siblings, 1 reply; 31+ messages in thread
From: Florian Weimer @ 2017-06-14 14:05 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> On 13/06/2017 11:24, Adhemerval Zanella wrote:
>
>> Here is an updated patch for the specialized dynarray. I have incorporated all
>> your suggestion but the skeleton name change (which I am not sure if it should
>> follow the idea since it should be a complete 'api'). I also added some nonull
>> attribute as for dynarray implementation.
>
> I rebase against the new begin/end additions and fixed some issues
> regarding the make check (which I forgot to actually run before patch
> submission...).
I don't see any begin/end references in the attached patch.
> +/* 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__ ((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)
> + return false;
> +
> + size_t newsize;
> + if (check_add_wrapv_size_t (pos, len, &newsize))
> + __libc_dynarray_overflow_failure (pos, len);
> + if (check_add_wrapv_size_t (newsize, 1, &newsize))
> + __libc_dynarray_overflow_failure (newsize, 1);
This is the opposite of what I expect: pos > array->dynarray_header.used
appears to be a usage error, so this could result in __libc_fatal.
Integer overflow while computing sizes for memory allocation is usually
treated as a memory allocation failure, so it would expect a false
return (and no __libc_fatal) for that.
If you want to prevent access to the underlying char_array_* functions
generated by dynarray, you could use #pragma GCC poison.
Florian
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/17] malloc: Add specialized dynarray for C strings
2017-06-14 14:04 ` Adhemerval Zanella
@ 2017-06-14 14:06 ` Florian Weimer
0 siblings, 0 replies; 31+ messages in thread
From: Florian Weimer @ 2017-06-14 14:06 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> On 14/06/2017 10:35, Florian Weimer wrote:
>> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>>
>>>>> +++ b/malloc/char_array.c
>>>>
>>>> Should this be malloc/char_array-skeleton.c, to indicate that this file
>>>> is parameterized and intended for inclusion into another .c file?
>>>
>>> I am not sure, the idea of char_array is to be self-contained, there is
>>> no need to define anything in the file to include it. Also, if we see
>>> other usage of dynamic C string inside glibc it would be a good idea to
>>> add internal symbols for common symbols.
>>
>> But it's still necessary to #include the file because it is what the C++
>> people call a header-only library. And there is a hook for customizing
>> the size of the stack allocation. This is why I asked. I don't have a
>> strong opinion on file naming, though.
>
> Fair enough then, although if this kind of usage pattern for dynarray
> become common it would be interesting to move to internal symbol. I will
> change to char_array-skeleton.c.
Agreed. In this case, we'd switch away from static functions, too.
Thanks,
Florian
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 06/17] posix: Rewrite to use struct scratch_buffer instead of extend_alloca
2017-06-08 21:14 ` [PATCH 06/17] posix: Rewrite to use struct scratch_buffer instead of extend_alloca Adhemerval Zanella
@ 2017-06-14 14:08 ` Florian Weimer
0 siblings, 0 replies; 31+ messages in thread
From: Florian Weimer @ 2017-06-14 14:08 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> From: Florian Weimer <fweimer@redhat.com>
>
> This patch removes a lot of boilerplate code to manager buffers for
> getpwnam_r.
>
> Checked on x86_64-linux-gnu.
>
> [BZ #18023]
> * posix/glob.c (glob): Use struct scratch_buffer instead of
> extend_alloca.
Still looks good to me.
Thanks,
Florian
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/17] malloc: Add specialized dynarray for C strings
2017-06-14 14:05 ` Florian Weimer
@ 2017-06-14 18:43 ` Adhemerval Zanella
2017-06-14 19:46 ` Florian Weimer
0 siblings, 1 reply; 31+ messages in thread
From: Adhemerval Zanella @ 2017-06-14 18:43 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
On 14/06/2017 11:05, Florian Weimer wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>
>> On 13/06/2017 11:24, Adhemerval Zanella wrote:
>>
>>> Here is an updated patch for the specialized dynarray. I have incorporated all
>>> your suggestion but the skeleton name change (which I am not sure if it should
>>> follow the idea since it should be a complete 'api'). I also added some nonull
>>> attribute as for dynarray implementation.
>>
>> I rebase against the new begin/end additions and fixed some issues
>> regarding the make check (which I forgot to actually run before patch
>> submission...).
>
> I don't see any begin/end references in the attached patch.
My mistake, I did not commit the two smalls changes that replace char_array_at (0)
with char_array_begin.
>
>> +/* 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__ ((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)
>> + return false;
>> +
>> + size_t newsize;
>> + if (check_add_wrapv_size_t (pos, len, &newsize))
>> + __libc_dynarray_overflow_failure (pos, len);
>> + if (check_add_wrapv_size_t (newsize, 1, &newsize))
>> + __libc_dynarray_overflow_failure (newsize, 1);
>
> This is the opposite of what I expect: pos > array->dynarray_header.used
> appears to be a usage error, so this could result in __libc_fatal.
> Integer overflow while computing sizes for memory allocation is usually
> treated as a memory allocation failure, so it would expect a false
> return (and no __libc_fatal) for that.
So I think a better approach would just to use:
if (pos > array->dynarray_header.used)
__libc_dynarray_at_failure (char_array_size (array), pos);
if (check_add_wrapv_size_t (pos, len, &newsize))
||check_add_wrapv_size_t (newsize, 1, &newsize))
return false;
>
> If you want to prevent access to the underlying char_array_* functions
> generated by dynarray, you could use #pragma GCC poison.
It could be a nice idea, although there is no direct usage on the glob
patchset. I will check this out for a future enhancement.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 07/17] malloc: Add specialized dynarray for C strings
2017-06-14 18:43 ` Adhemerval Zanella
@ 2017-06-14 19:46 ` Florian Weimer
0 siblings, 0 replies; 31+ messages in thread
From: Florian Weimer @ 2017-06-14 19:46 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>> This is the opposite of what I expect: pos > array->dynarray_header.used
>> appears to be a usage error, so this could result in __libc_fatal.
>> Integer overflow while computing sizes for memory allocation is usually
>> treated as a memory allocation failure, so it would expect a false
>> return (and no __libc_fatal) for that.
>
> So I think a better approach would just to use:
>
> if (pos > array->dynarray_header.used)
> __libc_dynarray_at_failure (char_array_size (array), pos);
>
> if (check_add_wrapv_size_t (pos, len, &newsize))
> ||check_add_wrapv_size_t (newsize, 1, &newsize))
> return false;
Right, this is what I would expect from such an interface. This assumes
that glob doesn't expect to feed bad existing indexes to this function,
of course.
>> If you want to prevent access to the underlying char_array_* functions
>> generated by dynarray, you could use #pragma GCC poison.
> It could be a nice idea, although there is no direct usage on the glob
> patchset. I will check this out for a future enhancement.
Understood.
Thanks,
Florian
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2017-06-14 19:46 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-08 21:13 [PATCH 00/17] posix: glob fixes and refactor Adhemerval Zanella
2017-06-08 21:13 ` [PATCH 02/17] posix: Allow glob to match dangling symlinks [BZ #866] Adhemerval Zanella
2017-06-08 21:13 ` [PATCH 03/17] support: Add optstring support Adhemerval Zanella
2017-06-13 9:22 ` Florian Weimer
2017-06-13 11:55 ` Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 09/17] posix: Remove glob GET_LOGIN_NAME_MAX usage Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 08/17] posix: Use char_array for internal glob dirname Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 01/17] posix: Sync glob with gnulib [BZ #1062] Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 07/17] malloc: Add specialized dynarray for C strings Adhemerval Zanella
2017-06-13 9:46 ` Florian Weimer
2017-06-13 12:17 ` Adhemerval Zanella
2017-06-13 14:24 ` Adhemerval Zanella
2017-06-13 20:51 ` Adhemerval Zanella
2017-06-14 14:05 ` Florian Weimer
2017-06-14 18:43 ` Adhemerval Zanella
2017-06-14 19:46 ` Florian Weimer
2017-06-14 13:35 ` Florian Weimer
2017-06-14 14:04 ` Adhemerval Zanella
2017-06-14 14:06 ` Florian Weimer
2017-06-08 21:14 ` [PATCH 06/17] posix: Rewrite to use struct scratch_buffer instead of extend_alloca Adhemerval Zanella
2017-06-14 14:08 ` Florian Weimer
2017-06-08 21:14 ` [PATCH 16/17] posix: Add common function to get home directory Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 04/17] posix: Adjust glob tests to libsupport Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 15/17] posix: Use char_array for home_dir in glob Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 12/17] posix: Remove alloca usage on glob dirname Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 14/17] posix: Remove all alloca usage in glob Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 05/17] posix: Consolidate glob implementation Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 11/17] posix: Remove alloca usage for GLOB_BRACE on glob Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 10/17] posix: User LOGIN_NAME_MAX for all user name in glob Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 17/17] posix: More check for overflow allocation " Adhemerval Zanella
2017-06-08 21:14 ` [PATCH 13/17] posix: Use dynarray for globname " 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).