* [PATCH 4/8] posix: Remove alloca usage on glob dirname
2017-11-21 13:55 [PATCH 0/8] posix: glob fixes and refactor Adhemerval Zanella
@ 2017-11-21 13:55 ` Adhemerval Zanella
2017-11-21 13:55 ` [PATCH 5/8] posix: Use dynarray for globname in glob Adhemerval Zanella
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2017-11-21 13:55 UTC (permalink / raw)
To: libc-alpha
This patch replaces the alloca/malloc usage for dirname creation
by the char_array struct.
Checked on x86_64-linux-gnu.
* posix/glob.c (glob_in_dir): Remove alloca usage for fullname.
Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
ChangeLog | 2 ++
posix/glob.c | 28 +++++++++-------------------
2 files changed, 11 insertions(+), 19 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index 7c0df0b..c83954d 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -1197,7 +1197,6 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
int (*errfunc) (const char *, int),
glob_t *pglob, size_t alloca_used)
{
- size_t dirlen = strlen (directory);
void *stream = NULL;
# define GLOBNAMES_MEMBERS(nnames) \
struct globnames *next; size_t count; char *name[nnames];
@@ -1229,32 +1228,23 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
}
else if (meta == GLOBPAT_NONE)
{
- size_t patlen = strlen (pattern);
- size_t fullsize;
- bool alloca_fullname
- = (! size_add_wrapv (dirlen + 1, patlen + 1, &fullsize)
- && glob_use_alloca (alloca_used, fullsize));
- char *fullname;
- if (alloca_fullname)
- fullname = alloca_account (fullsize, alloca_used);
- else
+ struct char_array fullname;
+
+ if (!char_array_init_str (&fullname, directory)
+ || !char_array_append_str (&fullname, "/")
+ || !char_array_append_str (&fullname, pattern))
{
- fullname = malloc (fullsize);
- if (fullname == NULL)
- return GLOB_NOSPACE;
+ char_array_free (&fullname);
+ return GLOB_NOSPACE;
}
- mempcpy (mempcpy (mempcpy (fullname, directory, dirlen),
- "/", 1),
- pattern, patlen + 1);
- if (glob_lstat (pglob, flags, fullname) == 0
+ if (glob_lstat (pglob, flags, char_array_str (&fullname)) == 0
|| errno == EOVERFLOW)
/* We found this file to be existing. Now tell the rest
of the function to copy this name into the result. */
flags |= GLOB_NOCHECK;
- if (__glibc_unlikely (!alloca_fullname))
- free (fullname);
+ char_array_free (&fullname);
}
else
{
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 5/8] posix: Use dynarray for globname in glob
2017-11-21 13:55 [PATCH 0/8] posix: glob fixes and refactor Adhemerval Zanella
2017-11-21 13:55 ` [PATCH 4/8] posix: Remove alloca usage on glob dirname Adhemerval Zanella
@ 2017-11-21 13:55 ` Adhemerval Zanella
2017-11-21 13:55 ` [PATCH 2/8] posix: Use char_array for internal glob dirname Adhemerval Zanella
` (6 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2017-11-21 13:55 UTC (permalink / raw)
To: libc-alpha
This patch uses dynarray at glob internal glob_in_dir function to manage
the various matched patterns. It simplify and removes all the boilerplate
buffer managements required. It also removes the glob_use_alloca, since
it is not used anymore.
Checked on x86_64-linux-gnu.
* posix/glob.c (glob_in_dir): Use dynarray for globnames.
---
ChangeLog | 2 +
posix/glob.c | 127 +++++++++++++++++------------------------------------------
2 files changed, 39 insertions(+), 90 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index c83954d..59baf62 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -1188,6 +1188,21 @@ prefix_array (const char *dirname, char **array, size_t n)
return 0;
}
+struct globnames_result
+{
+ char **names;
+ size_t length;
+};
+
+/* Create a dynamic array for C string representing the glob name found. */
+#define DYNARRAY_STRUCT globnames_array
+#define DYNARRAY_ELEMENT_FREE(ptr) free (*ptr)
+#define DYNARRAY_ELEMENT char *
+#define DYNARRAY_PREFIX globnames_array_
+#define DYNARRAY_FINAL_TYPE struct globnames_result
+#define DYNARRAY_INITIAL_SIZE 64
+#include <malloc/dynarray-skeleton.c>
+
/* Like 'glob', but PATTERN is a final pathname component,
and matches are searched for in DIRECTORY.
The GLOB_NOSORT bit in FLAGS is ignored. No sorting is ever done.
@@ -1198,25 +1213,13 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
glob_t *pglob, size_t alloca_used)
{
void *stream = NULL;
-# define GLOBNAMES_MEMBERS(nnames) \
- struct globnames *next; size_t count; char *name[nnames];
- struct globnames { GLOBNAMES_MEMBERS (FLEXIBLE_ARRAY_MEMBER) };
- struct { GLOBNAMES_MEMBERS (64) } init_names_buf;
- struct globnames *init_names = (struct globnames *) &init_names_buf;
- struct globnames *names = init_names;
- struct globnames *names_alloca = init_names;
+ struct globnames_array globnames;
size_t nfound = 0;
- size_t cur = 0;
int meta;
int save;
int result;
- alloca_used += sizeof init_names_buf;
-
- init_names->next = NULL;
- init_names->count = ((sizeof init_names_buf
- - offsetof (struct globnames, name))
- / sizeof init_names->name[0]);
+ globnames_array_init (&globnames);
meta = __glob_pattern_type (pattern, !(flags & GLOB_NOESCAPE));
if (meta == GLOBPAT_NONE && (flags & (GLOB_NOCHECK|GLOB_NOMAGIC)))
@@ -1293,34 +1296,10 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
if (fnmatch (pattern, d.name, fnm_flags) == 0)
{
- if (cur == names->count)
- {
- struct globnames *newnames;
- size_t count = names->count * 2;
- size_t nameoff = offsetof (struct globnames, name);
- size_t size = FLEXSIZEOF (struct globnames, name,
- count * sizeof (char *));
- if ((SIZE_MAX - nameoff) / 2 / sizeof (char *)
- < names->count)
- goto memory_error;
- if (glob_use_alloca (alloca_used, size))
- newnames = names_alloca
- = alloca_account (size, alloca_used);
- else if ((newnames = malloc (size))
- == NULL)
- goto memory_error;
- newnames->count = count;
- newnames->next = names;
- names = newnames;
- cur = 0;
- }
- names->name[cur] = strdup (d.name);
- if (names->name[cur] == NULL)
- goto memory_error;
- ++cur;
- ++nfound;
- if (SIZE_MAX - pglob->gl_offs <= nfound)
+ globnames_array_add (&globnames, strdup (d.name));
+ if (globnames_array_has_failed (&globnames))
goto memory_error;
+ nfound++;
}
}
}
@@ -1330,10 +1309,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;
@@ -1354,59 +1336,24 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
if (new_gl_pathv == NULL)
{
memory_error:
- while (1)
- {
- struct globnames *old = names;
- for (size_t i = 0; i < cur; ++i)
- free (names->name[i]);
- names = names->next;
- /* NB: we will not leak memory here if we exit without
- freeing the current block assigned to OLD. At least
- the very first block is always allocated on the stack
- and this is the block assigned to OLD here. */
- if (names == NULL)
- {
- assert (old == init_names);
- break;
- }
- cur = names->count;
- if (old == names_alloca)
- names_alloca = names;
- else
- free (old);
- }
+ globnames_array_free (&globnames);
result = GLOB_NOSPACE;
}
else
{
- while (1)
+ struct globnames_result ret = { .names = 0, .length = -1 };
+ if (!globnames_array_finalize (&globnames, &ret))
+ result = GLOB_NOSPACE;
+ else
{
- struct globnames *old = names;
- for (size_t i = 0; i < cur; ++i)
+ for (size_t i = 0; i < ret.length; ++i)
new_gl_pathv[pglob->gl_offs + pglob->gl_pathc++]
- = names->name[i];
- names = names->next;
- /* NB: we will not leak memory here if we exit without
- freeing the current block assigned to OLD. At least
- the very first block is always allocated on the stack
- and this is the block assigned to OLD here. */
- if (names == NULL)
- {
- assert (old == init_names);
- break;
- }
- cur = names->count;
- if (old == names_alloca)
- names_alloca = names;
- else
- free (old);
+ = ret.names[i];
+ pglob->gl_pathv = new_gl_pathv;
+ pglob->gl_pathv[pglob->gl_offs + pglob->gl_pathc] = NULL;
+ pglob->gl_flags = flags;
}
-
- pglob->gl_pathv = new_gl_pathv;
-
- pglob->gl_pathv[pglob->gl_offs + pglob->gl_pathc] = NULL;
-
- pglob->gl_flags = flags;
+ free (ret.names);
}
}
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 2/8] posix: Use char_array for internal glob dirname
2017-11-21 13:55 [PATCH 0/8] posix: glob fixes and refactor Adhemerval Zanella
2017-11-21 13:55 ` [PATCH 4/8] posix: Remove alloca usage on glob dirname Adhemerval Zanella
2017-11-21 13:55 ` [PATCH 5/8] posix: Use dynarray for globname in glob Adhemerval Zanella
@ 2017-11-21 13:55 ` Adhemerval Zanella
2017-11-21 13:55 ` [PATCH 3/8] posix: Remove alloca usage for GLOB_BRACE on glob Adhemerval Zanella
` (5 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2017-11-21 13:55 UTC (permalink / raw)
To: libc-alpha
This is the first patch of the set to remove alloca usage on glob
implementation. Internal path to search for file might expand to a
non static directory derived from pattern for some difference cases
(GLOB_NOESCAPE, GNU GLOB_TILDE) and to allow a non-static dirname
path glob uses a lot of boilerplate code to manage the buffer (which
is either allocated using alloca or malloc depending both to size
requested and the total alloca_used).
The patch changes to use the char_array struct with the default size
(256 bytes). It simplifies all the allocation code by using char_array
one and every internal buffer access is done using char_array provided
functions. No functional changes are expected.
Checked on x86_64-linux-gnu.
* posix/globc.c (glob): Use char_array for dirname.
Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
ChangeLog | 2 +
posix/glob.c | 270 ++++++++++++++++++++++++-----------------------------------
2 files changed, 112 insertions(+), 160 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index cb39779..7a89d2f 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -77,6 +77,7 @@
#include <flexmember.h>
#include <glob_internal.h>
#include <scratch_buffer.h>
+#include <malloc/char_array-skeleton.c>
\f
static const char *next_brace_sub (const char *begin, int flags) __THROWNL;
@@ -288,16 +289,15 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
glob_t *pglob)
{
const char *filename;
- char *dirname = NULL;
size_t dirlen;
int status;
size_t oldcount;
int meta;
- int dirname_modified;
- int malloc_dirname = 0;
+ bool dirname_modified;
glob_t dirs;
int retval = 0;
size_t alloca_used = 0;
+ struct char_array dirname;
if (pattern == NULL || pglob == NULL || (flags & ~__GLOB_FLAGS) != 0)
{
@@ -305,6 +305,10 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
return -1;
}
+ /* Default char array is stack allocated, so there is no need to check
+ if setting the initial '\0' succeeds. */
+ char_array_init_empty (&dirname);
+
/* POSIX requires all slashes to be matched. This means that with
a trailing slash we must match only directories. */
if (pattern[0] && pattern[strlen (pattern) - 1] == '/')
@@ -325,12 +329,12 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
size_t i;
if (pglob->gl_offs >= ~((size_t) 0) / sizeof (char *))
- return GLOB_NOSPACE;
+ goto err_nospace;
pglob->gl_pathv = (char **) malloc ((pglob->gl_offs + 1)
* sizeof (char *));
if (pglob->gl_pathv == NULL)
- return GLOB_NOSPACE;
+ goto err_nospace;
for (i = 0; i <= pglob->gl_offs; ++i)
pglob->gl_pathv[i] = NULL;
@@ -382,7 +386,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
{
onealt = malloc (pattern_len);
if (onealt == NULL)
- return GLOB_NOSPACE;
+ goto err_nospace;
}
/* We know the prefix for all sub-patterns. */
@@ -444,7 +448,8 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
globfree (pglob);
pglob->gl_pathc = 0;
}
- return result;
+ retval = result;
+ goto out;
}
if (*next == '}')
@@ -461,9 +466,10 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (pglob->gl_pathc != firstc)
/* We found some entries. */
- return 0;
+ retval = 0;
else if (!(flags & (GLOB_NOCHECK|GLOB_NOMAGIC)))
- return GLOB_NOMATCH;
+ retval = GLOB_NOMATCH;
+ goto out;
}
}
@@ -482,14 +488,15 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
filename = strchr (pattern, ':');
#endif /* __MSDOS__ || WINDOWS32 */
- dirname_modified = 0;
+ dirname_modified = false;
if (filename == NULL)
{
/* This can mean two things: a simple name or "~name". The latter
case is nothing but a notation for a directory. */
if ((flags & (GLOB_TILDE|GLOB_TILDE_CHECK)) && pattern[0] == '~')
{
- dirname = (char *) pattern;
+ if (!char_array_set_str (&dirname, pattern))
+ goto err_nospace;
dirlen = strlen (pattern);
/* Set FILENAME to NULL as a special flag. This is ugly but
@@ -506,7 +513,8 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
filename = pattern;
- dirname = (char *) ".";
+ if (!char_array_set_str (&dirname, "."))
+ goto err_nospace;
dirlen = 0;
}
}
@@ -515,13 +523,13 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
&& (flags & GLOB_NOESCAPE) == 0))
{
/* "/pattern" or "\\/pattern". */
- dirname = (char *) "/";
+ if (!char_array_set_str (&dirname, "/"))
+ goto err_nospace;
dirlen = 1;
++filename;
}
else
{
- char *newp;
dirlen = filename - pattern;
#if defined __MSDOS__ || defined WINDOWS32
if (*filename == ':'
@@ -535,31 +543,25 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
/* For now, disallow wildcards in the drive spec, to
prevent infinite recursion in glob. */
if (__glob_pattern_p (drive_spec, !(flags & GLOB_NOESCAPE)))
- return GLOB_NOMATCH;
+ {
+ retval = GLOB_NOMATCH;
+ goto out;
+ }
/* If this is "d:pattern", we need to copy ':' to DIRNAME
as well. If it's "d:/pattern", don't remove the slash
from "d:/", since "d:" and "d:/" are not the same.*/
}
#endif
-
- if (glob_use_alloca (alloca_used, dirlen + 1))
- newp = alloca_account (dirlen + 1, alloca_used);
- else
- {
- newp = malloc (dirlen + 1);
- if (newp == NULL)
- return GLOB_NOSPACE;
- malloc_dirname = 1;
- }
- *((char *) mempcpy (newp, pattern, dirlen)) = '\0';
- dirname = newp;
+ if (!char_array_set_str_size (&dirname, pattern, dirlen))
+ goto err_nospace;
++filename;
#if defined __MSDOS__ || defined WINDOWS32
bool drive_root = (dirlen > 1
- && (dirname[dirlen - 1] == ':'
- || (dirlen > 2 && dirname[dirlen - 2] == ':'
- && dirname[dirlen - 1] == '/')));
+ && (char_array_pos (&dirname, dirlen - 1) != ':'
+ || (dirlen > 2
+ && char_array_pos (&dirname, dirlen - 2) != ':'
+ && char_array_pos (&dirname, dirlen - 1) != '/')));
#else
bool drive_root = false;
#endif
@@ -568,20 +570,24 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
/* "pattern/". Expand "pattern", appending slashes. */
{
int orig_flags = flags;
- if (!(flags & GLOB_NOESCAPE) && dirname[dirlen - 1] == '\\')
+ if (!(flags & GLOB_NOESCAPE)
+ && char_array_pos (&dirname, dirlen - 1) == '\\')
{
/* "pattern\\/". Remove the final backslash if it hasn't
been quoted. */
- char *p = (char *) &dirname[dirlen - 1];
-
- while (p > dirname && p[-1] == '\\') --p;
- if ((&dirname[dirlen] - p) & 1)
+ size_t p = dirlen - 1;
+ while (p > 0 && char_array_pos (&dirname, p - 1) == '\\') --p;
+ if ((dirlen - p) & 1)
{
- *(char *) &dirname[--dirlen] = '\0';
+ /* Since we are shrinking the array, there is no need to
+ check the function return. */
+ dirlen -= 1;
+ char_array_crop (&dirname, dirlen);
flags &= ~(GLOB_NOCHECK | GLOB_NOMAGIC);
}
}
- int val = __glob (dirname, flags | GLOB_MARK, errfunc, pglob);
+ int val = __glob (char_array_str (&dirname), flags | GLOB_MARK,
+ errfunc, pglob);
if (val == 0)
pglob->gl_flags = ((pglob->gl_flags & ~GLOB_MARK)
| (flags & GLOB_MARK));
@@ -598,11 +604,14 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
}
- if ((flags & (GLOB_TILDE|GLOB_TILDE_CHECK)) && dirname[0] == '~')
+ if ((flags & (GLOB_TILDE|GLOB_TILDE_CHECK))
+ && char_array_pos (&dirname, 0) == '~')
{
- if (dirname[1] == '\0' || dirname[1] == '/'
- || (!(flags & GLOB_NOESCAPE) && dirname[1] == '\\'
- && (dirname[2] == '\0' || dirname[2] == '/')))
+ if (char_array_pos (&dirname, 1) == '\0'
+ || char_array_pos (&dirname, 1) == '/'
+ || (!(flags & GLOB_NOESCAPE) && char_array_pos (&dirname, 1) == '\\'
+ && (char_array_pos (&dirname, 2) == '\0'
+ || char_array_pos (&dirname, 2) == '/')))
{
/* Look up home directory. */
char *home_dir = getenv ("HOME");
@@ -652,10 +661,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (err != ERANGE)
break;
if (!scratch_buffer_grow (&s))
- {
- retval = GLOB_NOSPACE;
- goto out;
- }
+ goto err_nospace;
}
if (err == 0)
{
@@ -664,10 +670,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
scratch_buffer_free (&s);
if (err == 0 && home_dir == NULL)
- {
- retval = GLOB_NOSPACE;
- goto out;
- }
+ goto err_nospace;
#endif /* WINDOWS32 */
}
if (home_dir == NULL || home_dir[0] == '\0')
@@ -686,53 +689,26 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
}
/* Now construct the full directory. */
- if (dirname[1] == '\0')
+ if (char_array_pos (&dirname, 1) == '\0')
{
- if (__glibc_unlikely (malloc_dirname))
- free (dirname);
-
- dirname = home_dir;
- dirlen = strlen (dirname);
- malloc_dirname = malloc_home_dir;
+ if (!char_array_set_str (&dirname, home_dir))
+ goto err_nospace;
+ dirlen = char_array_size (&dirname) - 1;
}
else
{
- char *newp;
- size_t home_len = strlen (home_dir);
- int use_alloca = glob_use_alloca (alloca_used, home_len + dirlen);
- if (use_alloca)
- newp = alloca_account (home_len + dirlen, alloca_used);
- else
- {
- newp = malloc (home_len + dirlen);
- if (newp == NULL)
- {
- if (__glibc_unlikely (malloc_home_dir))
- free (home_dir);
- retval = GLOB_NOSPACE;
- goto out;
- }
- }
-
- mempcpy (mempcpy (newp, home_dir, home_len),
- &dirname[1], dirlen);
-
- if (__glibc_unlikely (malloc_dirname))
- free (dirname);
-
- dirname = newp;
- dirlen += home_len - 1;
- malloc_dirname = !use_alloca;
-
- if (__glibc_unlikely (malloc_home_dir))
- free (home_dir);
+ /* Replaces '~' by the obtained HOME dir. */
+ char_array_erase (&dirname, 0);
+ if (!char_array_prepend_str (&dirname, home_dir))
+ goto err_nospace;
}
- dirname_modified = 1;
+ dirname_modified = true;
}
else
{
#ifndef WINDOWS32
- char *end_name = strchr (dirname, '/');
+ char *dirnamestr = char_array_at (&dirname, 0);
+ char *end_name = strchr (dirnamestr, '/');
char *user_name;
int malloc_user_name = 0;
char *unescape = NULL;
@@ -741,23 +717,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;
@@ -767,8 +743,8 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
if (unescape != NULL)
{
- char *p = mempcpy (newp, dirname + 1,
- unescape - dirname - 1);
+ char *p = mempcpy (newp, dirnamestr + 1,
+ unescape - dirnamestr - 1);
char *q = unescape;
while (q != end_name)
{
@@ -790,7 +766,8 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
*p = '\0';
}
else
- *((char *) mempcpy (newp, dirname + 1, end_name - dirname - 1))
+ *((char *) mempcpy (newp, dirnamestr + 1,
+ end_name - dirnamestr - 1))
= '\0';
user_name = newp;
}
@@ -824,32 +801,13 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
/* If we found a home directory use this. */
if (p != NULL)
{
- size_t home_len = strlen (p->pw_dir);
- size_t rest_len = end_name == NULL ? 0 : strlen (end_name);
-
- if (__glibc_unlikely (malloc_dirname))
- free (dirname);
- malloc_dirname = 0;
-
- if (glob_use_alloca (alloca_used, home_len + rest_len + 1))
- dirname = alloca_account (home_len + rest_len + 1,
- alloca_used);
- else
+ if (!char_array_set_str (&dirname, p->pw_dir))
{
- dirname = malloc (home_len + rest_len + 1);
- if (dirname == NULL)
- {
- scratch_buffer_free (&pwtmpbuf);
- retval = GLOB_NOSPACE;
- goto out;
- }
- malloc_dirname = 1;
+ scratch_buffer_free (&pwtmpbuf);
+ goto err_nospace;
}
- *((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
{
@@ -890,37 +848,32 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
goto nospace;
pglob->gl_pathv = new_gl_pathv;
- if (flags & GLOB_MARK && is_dir (dirname, flags, pglob))
+ if (flags & GLOB_MARK
+ && is_dir (char_array_str (&dirname), flags, pglob))
{
char *p;
pglob->gl_pathv[newcount] = malloc (dirlen + 2);
if (pglob->gl_pathv[newcount] == NULL)
goto nospace;
- p = mempcpy (pglob->gl_pathv[newcount], dirname, dirlen);
+ p = mempcpy (pglob->gl_pathv[newcount],
+ char_array_str (&dirname), dirlen);
p[0] = '/';
p[1] = '\0';
- if (__glibc_unlikely (malloc_dirname))
- free (dirname);
}
else
{
- if (__glibc_unlikely (malloc_dirname))
- pglob->gl_pathv[newcount] = dirname;
- else
- {
- pglob->gl_pathv[newcount] = strdup (dirname);
- if (pglob->gl_pathv[newcount] == NULL)
- goto nospace;
- }
+ pglob->gl_pathv[newcount] = strdup (char_array_str (&dirname));
+ if (pglob->gl_pathv[newcount] == NULL)
+ goto nospace;
}
pglob->gl_pathv[++newcount] = NULL;
++pglob->gl_pathc;
pglob->gl_flags = flags;
-
- return 0;
+ goto out;
}
- meta = __glob_pattern_type (dirname, !(flags & GLOB_NOESCAPE));
+ meta = __glob_pattern_type (char_array_str (&dirname),
+ !(flags & GLOB_NOESCAPE));
/* meta is 1 if correct glob pattern containing metacharacters.
If meta has bit (1 << 2) set, it means there was an unterminated
[ which we handle the same, using fnmatch. Broken unterminated
@@ -933,15 +886,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))
@@ -955,7 +908,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
dirs.gl_lstat = pglob->gl_lstat;
}
- status = __glob (dirname,
+ status = __glob (char_array_str (&dirname),
((flags & (GLOB_ERR | GLOB_NOESCAPE | GLOB_ALTDIRFUNC))
| GLOB_NOSORT | GLOB_ONLYDIR),
errfunc, &dirs);
@@ -1002,8 +955,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
globfree (&dirs);
globfree (pglob);
pglob->gl_pathc = 0;
- retval = GLOB_NOSPACE;
- goto out;
+ goto err_nospace;
}
}
@@ -1025,8 +977,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
{
nospace2:
globfree (&dirs);
- retval = GLOB_NOSPACE;
- goto out;
+ goto err_nospace;
}
new_gl_pathv = realloc (pglob->gl_pathv,
@@ -1041,8 +992,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
globfree (&dirs);
globfree (pglob);
pglob->gl_pathc = 0;
- retval = GLOB_NOSPACE;
- goto out;
+ goto err_nospace;
}
++pglob->gl_pathc;
@@ -1068,7 +1018,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (meta & GLOBPAT_BACKSLASH)
{
- char *p = strchr (dirname, '\\'), *q;
+ char *p = strchr (char_array_str (&dirname), '\\'), *q;
/* We need to unescape the dirname string. It is certainly
allocated by alloca, as otherwise filename would be NULL
or dirname wouldn't contain backslashes. */
@@ -1085,12 +1035,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
@@ -1108,14 +1058,13 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (dirlen > 0)
{
/* Stick the directory on the front of each name. */
- if (prefix_array (dirname,
+ if (prefix_array (char_array_str (&dirname),
&pglob->gl_pathv[old_pathc + pglob->gl_offs],
pglob->gl_pathc - old_pathc))
{
globfree (pglob);
pglob->gl_pathc = 0;
- retval = GLOB_NOSPACE;
- goto out;
+ goto err_nospace;
}
}
}
@@ -1134,8 +1083,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
{
globfree (pglob);
pglob->gl_pathc = 0;
- retval = GLOB_NOSPACE;
- goto out;
+ goto err_nospace;
}
strcpy (&new[len - 2], "/");
pglob->gl_pathv[i] = new;
@@ -1151,10 +1099,12 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
out:
- if (__glibc_unlikely (malloc_dirname))
- free (dirname);
-
+ char_array_free (&dirname);
return retval;
+
+ err_nospace:
+ char_array_free (&dirname);
+ return GLOB_NOSPACE;
}
#if defined _LIBC && !defined __glob
versioned_symbol (libc, __glob, glob, GLIBC_2_27);
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 3/8] posix: Remove alloca usage for GLOB_BRACE on glob
2017-11-21 13:55 [PATCH 0/8] posix: glob fixes and refactor Adhemerval Zanella
` (2 preceding siblings ...)
2017-11-21 13:55 ` [PATCH 2/8] posix: Use char_array for internal glob dirname Adhemerval Zanella
@ 2017-11-21 13:55 ` Adhemerval Zanella
2017-11-21 13:55 ` [PATCH 8/8] posix: Remove all alloca usage in glob Adhemerval Zanella
` (4 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2017-11-21 13:55 UTC (permalink / raw)
To: libc-alpha
GNU GLOB_BRACE internal implementation constructs a new expression and
calls glob recursively. It then requires a possible large temporary
buffer place the new pattern.
This patch removes the alloca/malloc usage and replaces it with
char_array.
Checked on x86_64-linux-gnu.
* posix/glob.c (glob): Remove alloca usage for onealt.
Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
ChangeLog | 2 ++
posix/glob.c | 53 +++++++++++++++++++++++++++++------------------------
2 files changed, 31 insertions(+), 24 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index 7a89d2f..7c0df0b 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -372,25 +372,23 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
/* Allocate working buffer large enough for our work. Note that
we have at least an opening and closing brace. */
size_t firstc;
- char *alt_start;
const char *p;
const char *next;
const char *rest;
size_t rest_len;
- char *onealt;
- size_t pattern_len = strlen (pattern) - 1;
- int alloca_onealt = glob_use_alloca (alloca_used, pattern_len);
- if (alloca_onealt)
- onealt = alloca_account (pattern_len, alloca_used);
- else
- {
- onealt = malloc (pattern_len);
- if (onealt == NULL)
- goto err_nospace;
- }
+ struct char_array onealt;
/* We know the prefix for all sub-patterns. */
- alt_start = mempcpy (onealt, pattern, begin - pattern);
+ ptrdiff_t onealtlen = begin - pattern;
+ if (!char_array_init_str_size (&onealt, pattern, onealtlen))
+ {
+ if (!(flags & GLOB_APPEND))
+ {
+ pglob->gl_pathc = 0;
+ pglob->gl_pathv = NULL;
+ }
+ goto err_nospace;
+ }
/* Find the first sub-pattern and at the same time find the
rest after the closing brace. */
@@ -398,9 +396,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (next == NULL)
{
/* It is an invalid expression. */
- illegal_brace:
- if (__glibc_unlikely (!alloca_onealt))
- free (onealt);
+ char_array_free (&onealt);
flags &= ~GLOB_BRACE;
goto no_brace;
}
@@ -411,8 +407,12 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
{
rest = next_brace_sub (rest + 1, flags);
if (rest == NULL)
- /* It is an illegal expression. */
- goto illegal_brace;
+ {
+ /* It is an illegal expression. */
+ char_array_free (&onealt);
+ flags &= ~GLOB_BRACE;
+ goto no_brace;
+ }
}
/* Please note that we now can be sure the brace expression
is well-formed. */
@@ -431,9 +431,16 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
int result;
/* Construct the new glob expression. */
- mempcpy (mempcpy (alt_start, p, next - p), rest, rest_len);
+ ptrdiff_t nextlen = next - p;
+ if (!char_array_replace_str_pos (&onealt, onealtlen, p, nextlen)
+ || !char_array_replace_str_pos (&onealt, onealtlen + nextlen,
+ rest, rest_len))
+ {
+ char_array_free (&onealt);
+ goto err_nospace;
+ }
- result = __glob (onealt,
+ result = __glob (char_array_str (&onealt),
((flags & ~(GLOB_NOCHECK | GLOB_NOMAGIC))
| GLOB_APPEND),
errfunc, pglob);
@@ -441,8 +448,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
/* If we got an error, return it. */
if (result && result != GLOB_NOMATCH)
{
- if (__glibc_unlikely (!alloca_onealt))
- free (onealt);
+ char_array_free (&onealt);
if (!(flags & GLOB_APPEND))
{
globfree (pglob);
@@ -461,8 +467,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
assert (next != NULL);
}
- if (__glibc_unlikely (!alloca_onealt))
- free (onealt);
+ char_array_free (&onealt);
if (pglob->gl_pathc != firstc)
/* We found some entries. */
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 8/8] posix: Remove all alloca usage in glob
2017-11-21 13:55 [PATCH 0/8] posix: glob fixes and refactor Adhemerval Zanella
` (3 preceding siblings ...)
2017-11-21 13:55 ` [PATCH 3/8] posix: Remove alloca usage for GLOB_BRACE on glob Adhemerval Zanella
@ 2017-11-21 13:55 ` Adhemerval Zanella
2017-11-21 13:55 ` [PATCH 1/8] malloc: Add specialized dynarray for C strings Adhemerval Zanella
` (3 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2017-11-21 13:55 UTC (permalink / raw)
To: libc-alpha
With alloca usage removal from glob this patch wraps it up by removing
all the alloca defines and macros usage.
Checked on x86_64-linux-gnu.
* posix/glob.c (glob_in_dir, glob): Remove alloca_used argument.
Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
ChangeLog | 2 ++
posix/glob.c | 25 ++++++++-----------------
2 files changed, 10 insertions(+), 17 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index 71807d6..4d40c4c 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -32,7 +32,6 @@
#ifndef WINDOWS32
# include <pwd.h>
-# include <alloca.h>
#endif
#include <errno.h>
@@ -65,9 +64,6 @@
# define __stat64(fname, buf) stat (fname, buf)
# define __fxstatat64(_, d, f, st, flag) fstatat (d, f, st, flag)
# define struct_stat64 struct stat
-# ifndef __MVS__
-# define __alloca alloca
-# endif
# define __readdir readdir
# define COMPILE_GLOB64
#endif /* _LIBC */
@@ -174,12 +170,6 @@ convert_dirent64 (const struct dirent64 *source)
# ifdef GNULIB_defined_closedir
# undef closedir
# endif
-
-/* Just use malloc. */
-# define __libc_use_alloca(n) false
-# define alloca_account(len, avar) ((void) (len), (void) (avar), (void *) 0)
-# define extend_alloca_account(buf, len, newlen, avar) \
- ((void) (buf), (void) (len), (void) (newlen), (void) (avar), (void *) 0)
#endif
static int
@@ -207,7 +197,7 @@ glob_lstat (glob_t *pglob, int flags, const char *fullname)
static int glob_in_dir (const char *pattern, const char *directory,
int flags, int (*errfunc) (const char *, int),
- glob_t *pglob, size_t alloca_used);
+ glob_t *pglob);
static int prefix_array (const char *prefix, char **array, size_t n) __THROWNL;
static int collated_compare (const void *, const void *) __THROWNL;
@@ -273,7 +263,6 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
bool dirname_modified;
glob_t dirs;
int retval = 0;
- size_t alloca_used = 0;
struct char_array dirname;
if (pattern == NULL || pglob == NULL || (flags & ~__GLOB_FLAGS) != 0)
@@ -520,11 +509,13 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
char *drive_spec;
++dirlen;
- drive_spec = __alloca (dirlen + 1);
+ drive_spec = malloc (dirlen + 1);
*((char *) mempcpy (drive_spec, pattern, dirlen)) = '\0';
/* For now, disallow wildcards in the drive spec, to
prevent infinite recursion in glob. */
- if (__glob_pattern_p (drive_spec, !(flags & GLOB_NOESCAPE)))
+ int r = __glob_pattern_p (drive_spec, !(flags & GLOB_NOESCAPE));
+ free (drive_spec);
+ if (r != 0)
{
retval = GLOB_NOMATCH;
goto out;
@@ -920,7 +911,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;
@@ -1025,7 +1016,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
@@ -1190,7 +1181,7 @@ struct globnames_result
static int
glob_in_dir (const char *pattern, const char *directory, int flags,
int (*errfunc) (const char *, int),
- glob_t *pglob, size_t alloca_used)
+ glob_t *pglob)
{
void *stream = NULL;
struct globnames_array globnames;
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/8] malloc: Add specialized dynarray for C strings
2017-11-21 13:55 [PATCH 0/8] posix: glob fixes and refactor Adhemerval Zanella
` (4 preceding siblings ...)
2017-11-21 13:55 ` [PATCH 8/8] posix: Remove all alloca usage in glob Adhemerval Zanella
@ 2017-11-21 13:55 ` Adhemerval Zanella
2017-11-21 14:15 ` Andreas Schwab
2017-11-21 14:19 ` Andreas Schwab
2017-11-21 13:55 ` [PATCH 6/8] posix: Remove alloca usage on glob user_name Adhemerval Zanella
` (2 subsequent siblings)
8 siblings, 2 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2017-11-21 13:55 UTC (permalink / raw)
To: libc-alpha
This patch adds an specialized dynarray to manage C strings using the
dynarray internal implementation. It uses some private fields from
dynarray and thus it provided specific files to access and manage
the internal string buffer.
For instance:
struct char_array str;
// str == "testing"
char_array_init_str (&str, "testing");
// c == 's'
char c = char_array_pos (&str, 2);
// str = "testing2"
char_array_set_str (&str, "testing2");
// str = "testig2"
char_array_erase (&str, 5);
// str = "123testig2";
char_array_prepend_str (&str, "123");
// len = 10;
size_t len = char_array_length (&str);
// str = "123testig2456";
char_array_append_str (&str, "456");
// str = "123test789";
char_array_replace_str_pos (&str, 7, "789", 3);
The provided function are not extensive and meant mainly to be use in
subsequent glob implementation cleanup. For internal object consistency
only the function provided by char_array.c should be used, including
internal object manipulation.
To check for possible overflows in internal size manipulation a new
function, check_add_wrapv_size_t, is added on malloc-internal. It basically
return whether the addition of two size_t overflows.
Checked on x86_64-linux-gnu.
* malloc/Makefile (test-internal): Add tst-char_array.
(routines): Add dynarray_overflow_failure and char_array-impl.
* malloc/Versions [GLIBC_PRIVATE] (libc): Add
__libc_dynarray_overflow_failure, __char_array_set_str_size,
__char_array_erase, __char_array_prepend_str_size, and
__char_array_replace_str_pos.
* malloc/char_array-impl.c: New file.
* malloc/char_array-skeleton.c: Likewise.
* malloc/char_array.h: Likewise.
* malloc/tst-char-array.c: Likewise.
* malloc/dynarray_overflow_failure.c: Likewise.
* malloc/malloc-internal.h (check_add_overflow_size_t): New function.
Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
ChangeLog | 15 ++
malloc/Makefile | 4 +-
malloc/Versions | 7 +
malloc/char_array-impl.c | 57 ++++++++
malloc/char_array-skeleton.c | 279 +++++++++++++++++++++++++++++++++++++
malloc/char_array.h | 53 +++++++
malloc/dynarray.h | 9 ++
malloc/dynarray_overflow_failure.c | 31 +++++
malloc/malloc-internal.h | 14 ++
malloc/tst-char_array.c | 112 +++++++++++++++
10 files changed, 580 insertions(+), 1 deletion(-)
create mode 100644 malloc/char_array-impl.c
create mode 100644 malloc/char_array-skeleton.c
create mode 100644 malloc/char_array.h
create mode 100644 malloc/dynarray_overflow_failure.c
create mode 100644 malloc/tst-char_array.c
diff --git a/malloc/Makefile b/malloc/Makefile
index 17936fc..8435e7f 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -49,6 +49,7 @@ tests-internal += \
tst-dynarray \
tst-dynarray-fail \
tst-dynarray-at-fail \
+ tst-char_array
ifneq (no,$(have-tunables))
tests += tst-malloc-usable-tunables
@@ -61,7 +62,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 \
@@ -71,6 +72,7 @@ routines = malloc morecore mcheck mtrace obstack reallocarray \
alloc_buffer_copy_bytes \
alloc_buffer_copy_string \
alloc_buffer_create_failure \
+ char_array-impl
install-lib := libmcheck.a
non-lib.a := libmcheck.a
diff --git a/malloc/Versions b/malloc/Versions
index 2357cff..b21fe59 100644
--- a/malloc/Versions
+++ b/malloc/Versions
@@ -81,6 +81,7 @@ libc {
# dynarray support
__libc_dynarray_at_failure;
+ __libc_dynarray_overflow_failure;
__libc_dynarray_emplace_enlarge;
__libc_dynarray_finalize;
__libc_dynarray_resize;
@@ -92,5 +93,11 @@ libc {
__libc_alloc_buffer_copy_bytes;
__libc_alloc_buffer_copy_string;
__libc_alloc_buffer_create_failure;
+
+ # char_array support
+ __char_array_set_str_size;
+ __char_array_erase;
+ __char_array_prepend_str_size;
+ __char_array_replace_str_pos;
}
}
diff --git a/malloc/char_array-impl.c b/malloc/char_array-impl.c
new file mode 100644
index 0000000..5a56bbc
--- /dev/null
+++ b/malloc/char_array-impl.c
@@ -0,0 +1,57 @@
+/* Specialized dynarray for C strings. Implementation file.
+ 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 <malloc/char_array.h>
+
+void
+__char_array_set_str_size (struct dynarray_header *header, const char *str,
+ size_t size)
+{
+ *((char *) mempcpy (header->array, str, size)) = '\0';
+ header->used = size + 1;
+}
+libc_hidden_def (__char_array_set_str_size)
+
+void
+__char_array_erase (struct dynarray_header *header, size_t pos)
+{
+ char *ppos = header->array + pos;
+ char *lpos = header->array + header->used;
+ ptrdiff_t size = lpos - ppos;
+ memmove (ppos, ppos + 1, size);
+ header->used--;
+}
+libc_hidden_def (__char_array_erase)
+
+void
+__char_array_prepend_str_size (struct dynarray_header *header,
+ const char *str, size_t size, size_t used)
+{
+ memmove (header->array + size, header->array, used);
+ memcpy (header->array, str, size);
+}
+libc_hidden_def (__char_array_prepend_str_size)
+
+void
+__char_array_replace_str_pos (struct dynarray_header *header, size_t pos,
+ const char *str, size_t len)
+{
+ char *start = header->array + pos;
+ *(char *) mempcpy (start, str, len) = '\0';
+}
+libc_hidden_def (__char_array_replace_str_pos)
diff --git a/malloc/char_array-skeleton.c b/malloc/char_array-skeleton.c
new file mode 100644
index 0000000..00d3a30
--- /dev/null
+++ b/malloc/char_array-skeleton.c
@@ -0,0 +1,279 @@
+/* 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 provides dynamic size
+ expansion and heap usage for large strings.
+
+ The following parameters are optional:
+
+ CHAR_ARRAY_INITIAL_SIZE
+ The size of the statically allocated array (default is 256). It will
+ be used to define DYNARRAY_INITIAL_SIZE.
+
+ The following functions are provided:
+
+ bool char_array_init_empty (struct char_array *);
+ bool char_array_init_str (struct char_array *, const char *);
+ bool char_array_init_str_size (struct char_array *, const char *, size_t);
+ bool char_array_is_empty (struct char_array *);
+ const char *char_array_str (struct char_array *);
+ char char_array_pos (struct char_array *, size_t);
+ size_t char_array_length (struct char_array *);
+ bool char_array_set_str (struct char_array *, const char *);
+ bool char_array_set_str_size (struct char_array *, const char *, size_t);
+ void char_array_erase (struct char_array *, size_t);
+ bool char_array_crop (struct char_array *, size_t);
+ bool char_array_prepend_str (struct char_array *, const char *);
+ bool char_array_append_str (struct char_array *, const char *);
+ bool char_array_replace_str_pos (struct char_array *, size_t, const char *,
+ size_t);
+
+ For instance:
+
+ struct char_array str;
+
+ // str == "testing"
+ char_array_init_str (&str, "testing");
+
+ // c == 's'
+ char c = char_array_pos (&str, 2);
+
+ // str = "testing2"
+ char_array_set_str (&str, "testing2");
+
+ // str = "testig2"
+ char_array_erase (&str, 5);
+
+ // str = "123testig2";
+ char_array_prepend_str (&str, "123");
+
+ // len = 10;
+ size_t len = char_array_length (&str);
+
+ // str = "123testig2456";
+ char_array_append_str (&str, "456");
+
+ // str = "123test789";
+ char_array_replace_str_pos (&str, 7, "789", 3);
+ */
+
+#define DYNARRAY_STRUCT char_array
+#define DYNARRAY_ELEMENT char
+#define DYNARRAY_PREFIX char_array_
+#ifndef CHAR_ARRAY_INITIAL_SIZE
+# define CHAR_ARRAY_INITIAL_SIZE 256
+#endif
+#define DYNARRAY_INITIAL_SIZE CHAR_ARRAY_INITIAL_SIZE
+#include <malloc/dynarray-skeleton.c>
+
+#include <malloc/malloc-internal.h>
+#include <malloc/char_array.h>
+
+/* Return a const char for the internal C string handled by 'array'. */
+__attribute__ ((unused, nonnull (1)))
+static const char *
+char_array_str (struct char_array *array)
+{
+ return char_array_begin (array);
+}
+
+/* Return the character at position 'pos' from the char_array 'array'. */
+__attribute__ ((unused, nonnull (1)))
+static char
+char_array_pos (struct char_array *array, size_t pos)
+{
+ return *char_array_at (array, pos);
+}
+
+/* Calculate the length of the string, excluding the terminating null. */
+__attribute__ ((unused, nonnull (1)))
+static size_t
+char_array_length (struct char_array *array)
+{
+ /* Exclude the final '\0'. */
+ return array->dynarray_header.used - 1;
+}
+
+/* Copy up 'size' bytes from string 'str' to char_array 'array'. A final
+ '\0' is appended in the char_array. */
+__attribute__ ((unused, nonnull (1, 2)))
+static bool
+char_array_set_str_size (struct char_array *array, const char *str,
+ size_t size)
+{
+ size_t newsize;
+ if (check_add_overflow_size_t (size, 1, &newsize))
+ __libc_dynarray_overflow_failure (size, 1);
+
+ if (!char_array_resize (array, newsize))
+ return false;
+
+ __char_array_set_str_size (&array->dynarray_abstract, str, size);
+ return true;
+}
+
+/* Copy the contents of string 'str' to char_array 'array', including the
+ final '\0'. */
+__attribute__ ((unused, nonnull (1, 2)))
+static bool
+char_array_set_str (struct char_array *array, const char *str)
+{
+ return char_array_set_str_size (array, str, strlen (str));
+}
+
+/* Initialize the char_array 'array' and sets it to an empty string (""). */
+__attribute__ ((unused, nonnull (1)))
+static bool
+char_array_init_empty (struct char_array *array)
+{
+ char_array_init (array);
+ return char_array_set_str (array, "");
+}
+
+/* Initialize the char_array 'array' and copy the content of string 'str'. */
+__attribute__ ((unused, nonnull (1, 2)))
+static bool
+char_array_init_str (struct char_array *array, const char *str)
+{
+ char_array_init (array);
+ return char_array_set_str (array, str);
+}
+
+/* Initialize the char_array 'array' and copy the content of string 'str'
+ up to 'size' characteres. */
+__attribute__ ((unused, nonnull (1, 2)))
+static bool
+char_array_init_str_size (struct char_array *array, const char *str,
+ size_t size)
+{
+ char_array_init (array);
+ return char_array_set_str_size (array, str, size);
+}
+
+/* Return if the char_array contain any characteres. */
+__attribute__ ((unused, nonnull (1)))
+static bool
+char_array_is_empty (struct char_array *array)
+{
+ return *char_array_begin (array) == '\0';
+}
+
+/* Remove the byte at position 'pos' from char_array 'array'. The contents
+ are moved internally if the position is not at the end of the internal
+ buffer. */
+__attribute__ ((unused, nonnull (1)))
+static bool
+char_array_erase (struct char_array *array, size_t pos)
+{
+ if (pos >= array->dynarray_header.used - 1)
+ return false;
+
+ __char_array_erase (&array->dynarray_abstract, pos);
+ return true;
+}
+
+/* Resize the char_array 'array' to size 'count' maintaining the ending
+ '\0' byte. */
+__attribute__ ((unused, nonnull (1)))
+static bool
+char_array_crop (struct char_array *array, size_t size)
+{
+ if (size >= (array->dynarray_header.used - 1)
+ || !char_array_resize (array, size + 1))
+ return false;
+
+ array->dynarray_header.array[size] = '\0';
+ return true;
+}
+
+/* Prepend the contents of string 'str' to char_array 'array', including the
+ final '\0' byte. */
+__attribute__ ((unused, nonnull (1, 2)))
+static bool
+char_array_prepend_str (struct char_array *array, const char *str)
+{
+ size_t size = strlen (str);
+ /* Resizing the array might change its used elements and we need below
+ to correct copy the elements. */
+ size_t used = array->dynarray_header.used;
+
+ size_t newsize;
+ if (check_add_overflow_size_t (used, size, &newsize))
+ __libc_dynarray_overflow_failure (used, size);
+
+ /* Make room for the string and copy it. */
+ if (!char_array_resize (array, newsize))
+ return false;
+ __char_array_prepend_str_size (&array->dynarray_abstract, str, size, used);
+ return true;
+}
+
+/* Append the contents of string 'str' to char_array 'array, including the
+ final '\0' byte. */
+__attribute__ ((unused, nonnull (1, 2)))
+static bool
+char_array_append_str (struct char_array *array, const char *str)
+{
+ size_t size = strlen (str);
+ /* Resizing the array might change its used elements and it used it below
+ to correct copy the elements. */
+ size_t used = array->dynarray_header.used - 1;
+
+ /* 'used' does account for final '\0', so there is no need to add
+ an extra element to calculate the final required size. */
+ size_t newsize;
+ if (check_add_overflow_size_t (used + 1, size, &newsize))
+ __libc_dynarray_overflow_failure (used + 1, size);
+
+ if (!char_array_resize (array, newsize))
+ return false;
+
+ /* Start to append at '\0' up to string length and add a final '\0'. */
+ *(char*) mempcpy (array->dynarray_header.array + used, str, size) = '\0';
+ return true;
+}
+
+/* Append the character 'c' on char_array 'array'. */
+__attribute__ ((unused, nonnull (1)))
+static bool
+char_array_append_char (struct char_array *array, char c)
+{
+ return char_array_append_str (array, (const char[]) { c, '\0' });
+}
+
+/* Replace the contents starting of position 'pos' of char_array 'array'
+ with the contents of string 'str' up to 'len' bytes. A final '\0'
+ is appended in the string. */
+__attribute__ ((unused, nonnull (1, 3)))
+static bool
+char_array_replace_str_pos (struct char_array *array, size_t pos,
+ const char *str, size_t len)
+{
+ if (pos > array->dynarray_header.used)
+ __libc_dynarray_at_failure (array->dynarray_header.used, pos);
+
+ size_t newsize;
+ if (check_add_overflow_size_t (pos, len, &newsize)
+ || check_add_overflow_size_t (newsize, 1, &newsize)
+ || !char_array_resize (array, newsize))
+ return false;
+
+ __char_array_replace_str_pos (&array->dynarray_abstract, pos, str, len);
+ return true;
+}
diff --git a/malloc/char_array.h b/malloc/char_array.h
new file mode 100644
index 0000000..c696673
--- /dev/null
+++ b/malloc/char_array.h
@@ -0,0 +1,53 @@
+/* Specialized dynarray for C strings. Shared definitions.
+ 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 _CHAR_ARRAY_H
+#define _CHAR_ARRAY_H
+
+#include <malloc/dynarray.h>
+
+/* Internal funciton. Set the dynarray to the content of the string STR up
+ to SIZE bytes. The dynarray must be resized previously. */
+void __char_array_set_str_size (struct dynarray_header *, const char *str,
+ size_t size);
+
+/* Internal function. Remove the character at position POS from dynarray.
+ The position must be a valid one. */
+void __char_array_erase (struct dynarray_header *, size_t pos);
+
+/* Internal function. Prepend the content of string STR up to SIZE bytes to
+ dynarray by moving USED bytes forward. The dynarray must be resized
+ previously. */
+void __char_array_prepend_str_size (struct dynarray_header *,
+ const char *str, size_t size,
+ size_t used);
+
+/* Internal function. Replace the content of dynarray starting at position
+ POS with the content of string STR up to LEN bytes. The dynarray must
+ be resize previously and STR must contain at least LEN bytes. */
+void __char_array_replace_str_pos (struct dynarray_header *, size_t pos,
+ const char *str, size_t len);
+
+#ifndef _ISOMAC
+libc_hidden_proto (__char_array_set_str_size)
+libc_hidden_proto (__char_array_erase)
+libc_hidden_proto (__char_array_prepend_str_size)
+libc_hidden_proto (__char_array_replace_str_pos)
+#endif
+
+#endif
diff --git a/malloc/dynarray.h b/malloc/dynarray.h
index 5888bcb..bb52b0f 100644
--- a/malloc/dynarray.h
+++ b/malloc/dynarray.h
@@ -168,12 +168,21 @@ bool __libc_dynarray_finalize (struct dynarray_header *list, void *scratch,
void __libc_dynarray_at_failure (size_t size, size_t index)
__attribute__ ((noreturn));
+/* Internal function. TErminate the process after an overflow in
+ new size allocation. SIZE is the current number of elements in
+ dynamic array and INCR is the new elements to add on current
+ size. */
+void __libc_dynarray_overflow_failure (size_t size, size_t incr)
+ __attribute__ ((noreturn));
+
#ifndef _ISOMAC
libc_hidden_proto (__libc_dynarray_emplace_enlarge)
libc_hidden_proto (__libc_dynarray_resize)
libc_hidden_proto (__libc_dynarray_resize_clear)
libc_hidden_proto (__libc_dynarray_finalize)
libc_hidden_proto (__libc_dynarray_at_failure)
+libc_hidden_proto (__libc_dynarray_overflow_failure)
+
#endif
#endif /* _DYNARRAY_H */
diff --git a/malloc/dynarray_overflow_failure.c b/malloc/dynarray_overflow_failure.c
new file mode 100644
index 0000000..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 a9c9c6a..3957ebb 100644
--- a/malloc/malloc-internal.h
+++ b/malloc/malloc-internal.h
@@ -91,4 +91,18 @@ check_mul_overflow_size_t (size_t left, size_t right, size_t *result)
#endif
}
+/* Set *R = A + B. Return true if the answer is mathematically incorrect due
+ to overflow; in this case, *R is the low order bits of the correct
+ answer. */
+static inline bool
+check_add_overflow_size_t (size_t a, size_t b, size_t *r)
+{
+#if 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..a3a99fb
--- /dev/null
+++ b/malloc/tst-char_array.c
@@ -0,0 +1,112 @@
+/* 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-skeleton.c>
+
+#include <malloc.h>
+#include <mcheck.h>
+#include <stdint.h>
+#include <support/check.h>
+#include <support/support.h>
+
+static int
+do_test (void)
+{
+ mtrace ();
+
+ {
+ struct char_array str;
+ TEST_VERIFY_EXIT (char_array_init_empty (&str) == true);
+ TEST_VERIFY_EXIT (char_array_length (&str) == 0);
+ TEST_VERIFY_EXIT (char_array_is_empty (&str) == true);
+ TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "") == 0);
+ char_array_free (&str);
+ }
+
+ {
+ struct char_array str;
+ TEST_VERIFY_EXIT (char_array_init_str (&str, "testing"));
+ TEST_VERIFY_EXIT (char_array_length (&str) == strlen ("testing"));
+ TEST_VERIFY_EXIT (char_array_pos (&str, 2) == 's');
+ TEST_VERIFY_EXIT (char_array_is_empty (&str) == false);
+ TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "testing") == 0);
+ char_array_free (&str);
+ }
+
+ {
+ struct char_array str;
+ TEST_VERIFY_EXIT (char_array_init_str_size (&str, "testing", 4));
+ TEST_VERIFY_EXIT (char_array_length (&str) == 4);
+ TEST_VERIFY_EXIT (char_array_pos (&str, 2) == 's');
+ TEST_VERIFY_EXIT (char_array_is_empty (&str) == false);
+ TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "test") == 0);
+ char_array_free (&str);
+ }
+
+ {
+ struct char_array str;
+ TEST_VERIFY_EXIT (char_array_init_str (&str, "testing"));
+ TEST_VERIFY_EXIT (char_array_set_str (&str, "abcdef"));
+ TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "abcdef") == 0);
+ TEST_VERIFY_EXIT (char_array_set_str_size (&str, "abcdef", 4));
+ TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "abcd") == 0);
+ char_array_free (&str);
+ }
+
+ {
+ struct char_array str;
+ TEST_VERIFY_EXIT (char_array_init_str (&str, "testing"));
+ TEST_VERIFY_EXIT (char_array_erase (&str, 4) == true);
+ TEST_VERIFY_EXIT (char_array_length (&str) == strlen ("testing") - 1);
+ TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "testng") == 0);
+ TEST_VERIFY_EXIT (char_array_erase (&str, char_array_length (&str))
+ == false);
+ TEST_VERIFY_EXIT (char_array_length (&str) == strlen ("testing") - 1);
+ TEST_VERIFY_EXIT (char_array_erase (&str, char_array_length (&str) - 1)
+ == true);
+ TEST_VERIFY_EXIT (char_array_length (&str) == strlen ("testing") - 2);
+ TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "testn") == 0);
+ char_array_free (&str);
+ }
+
+ {
+ struct char_array str;
+ TEST_VERIFY_EXIT (char_array_init_str (&str, "test"));
+ TEST_VERIFY_EXIT (char_array_prepend_str (&str, "123"));
+ TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "123test") == 0);
+ TEST_VERIFY_EXIT (char_array_length (&str) == strlen ("123test"));
+ TEST_VERIFY_EXIT (char_array_append_str (&str, "456"));
+ TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "123test456") == 0);
+ TEST_VERIFY_EXIT (char_array_length (&str) == strlen ("123test456"));
+ TEST_VERIFY_EXIT (char_array_replace_str_pos (&str, 7, "789", 3));
+ TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "123test789") == 0);
+ TEST_VERIFY_EXIT (char_array_length (&str) == strlen ("123test789"));
+ TEST_VERIFY_EXIT (char_array_crop (&str, 7));
+ TEST_VERIFY_EXIT (char_array_length (&str) == 7);
+ TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "123test") == 0);
+ TEST_VERIFY_EXIT (char_array_append_char (&str, '4'));
+ TEST_VERIFY_EXIT (strcmp (char_array_str (&str), "123test4") == 0);
+ char_array_free (&str);
+ }
+
+ return 0;
+}
+
+#include <support/test-driver.c>
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/8] malloc: Add specialized dynarray for C strings
2017-11-21 13:55 ` [PATCH 1/8] malloc: Add specialized dynarray for C strings Adhemerval Zanella
@ 2017-11-21 14:15 ` Andreas Schwab
2017-11-21 16:20 ` Adhemerval Zanella
2017-11-21 14:19 ` Andreas Schwab
1 sibling, 1 reply; 15+ messages in thread
From: Andreas Schwab @ 2017-11-21 14:15 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
On Nov 21 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> diff --git a/malloc/dynarray.h b/malloc/dynarray.h
> index 5888bcb..bb52b0f 100644
> --- a/malloc/dynarray.h
> +++ b/malloc/dynarray.h
> @@ -168,12 +168,21 @@ bool __libc_dynarray_finalize (struct dynarray_header *list, void *scratch,
> void __libc_dynarray_at_failure (size_t size, size_t index)
> __attribute__ ((noreturn));
>
> +/* Internal function. TErminate the process after an overflow in
Terminate
> diff --git a/malloc/malloc-internal.h b/malloc/malloc-internal.h
> index a9c9c6a..3957ebb 100644
> --- a/malloc/malloc-internal.h
> +++ b/malloc/malloc-internal.h
> @@ -91,4 +91,18 @@ check_mul_overflow_size_t (size_t left, size_t right, size_t *result)
> #endif
> }
>
> +/* Set *R = A + B. Return true if the answer is mathematically incorrect due
> + to overflow; in this case, *R is the low order bits of the correct
> + answer. */
> +static inline bool
> +check_add_overflow_size_t (size_t a, size_t b, size_t *r)
> +{
> +#if 5 <= __GNUC__
__GNUC__ >= 5
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 1/8] malloc: Add specialized dynarray for C strings
2017-11-21 14:15 ` Andreas Schwab
@ 2017-11-21 16:20 ` Adhemerval Zanella
0 siblings, 0 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2017-11-21 16:20 UTC (permalink / raw)
To: Andreas Schwab; +Cc: libc-alpha
On 21/11/2017 12:15, Andreas Schwab wrote:
> On Nov 21 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
>
>> diff --git a/malloc/dynarray.h b/malloc/dynarray.h
>> index 5888bcb..bb52b0f 100644
>> --- a/malloc/dynarray.h
>> +++ b/malloc/dynarray.h
>> @@ -168,12 +168,21 @@ bool __libc_dynarray_finalize (struct dynarray_header *list, void *scratch,
>> void __libc_dynarray_at_failure (size_t size, size_t index)
>> __attribute__ ((noreturn));
>>
>> +/* Internal function. TErminate the process after an overflow in
>
> Terminate
>
>> diff --git a/malloc/malloc-internal.h b/malloc/malloc-internal.h
>> index a9c9c6a..3957ebb 100644
>> --- a/malloc/malloc-internal.h
>> +++ b/malloc/malloc-internal.h
>> @@ -91,4 +91,18 @@ check_mul_overflow_size_t (size_t left, size_t right, size_t *result)
>> #endif
>> }
>>
>> +/* Set *R = A + B. Return true if the answer is mathematically incorrect due
>> + to overflow; in this case, *R is the low order bits of the correct
>> + answer. */
>> +static inline bool
>> +check_add_overflow_size_t (size_t a, size_t b, size_t *r)
>> +{
>> +#if 5 <= __GNUC__
>
> __GNUC__ >= 5
>
> Andreas.
>
I fixed both locally, thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/8] malloc: Add specialized dynarray for C strings
2017-11-21 13:55 ` [PATCH 1/8] malloc: Add specialized dynarray for C strings Adhemerval Zanella
2017-11-21 14:15 ` Andreas Schwab
@ 2017-11-21 14:19 ` Andreas Schwab
2017-11-21 16:32 ` Adhemerval Zanella
1 sibling, 1 reply; 15+ messages in thread
From: Andreas Schwab @ 2017-11-21 14:19 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
On Nov 21 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> // c == 's'
> char c = char_array_pos (&str, 2);
I'd call it char_array_at.
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/8] malloc: Add specialized dynarray for C strings
2017-11-21 14:19 ` Andreas Schwab
@ 2017-11-21 16:32 ` Adhemerval Zanella
0 siblings, 0 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2017-11-21 16:32 UTC (permalink / raw)
To: Andreas Schwab; +Cc: libc-alpha
On 21/11/2017 12:19, Andreas Schwab wrote:
> On Nov 21 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
>
>> // c == 's'
>> char c = char_array_pos (&str, 2);
>
> I'd call it char_array_at.
>
> Andreas.
>
Since char_array is based directly on dynarray the name is already implemented
by it. In fact we do have a char_array_at, but it returns a 'char *' to the
position.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/8] posix: Remove alloca usage on glob user_name
2017-11-21 13:55 [PATCH 0/8] posix: glob fixes and refactor Adhemerval Zanella
` (5 preceding siblings ...)
2017-11-21 13:55 ` [PATCH 1/8] malloc: Add specialized dynarray for C strings Adhemerval Zanella
@ 2017-11-21 13:55 ` Adhemerval Zanella
2017-11-21 13:55 ` [PATCH 7/8] posix: Use char_array for home_dir in glob Adhemerval Zanella
2017-12-12 17:02 ` [PATCH 0/8] posix: glob fixes and refactor Adhemerval Zanella
8 siblings, 0 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2017-11-21 13:55 UTC (permalink / raw)
To: libc-alpha
This patch uses dynarray at glob internal user name manipulation for
GLOB_TILDE. It simplifies it and removes all the boilerplate buffer
managements required. It also removes the glob_use_alloca, since
it is not used anymore.
Checked on x86_64-linux-gnu.
* posix/glob.c (size_add_wrapv, glob_use_alloca): Remove.
(__glob): Use char_array for user name.
Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
ChangeLog | 3 +++
posix/glob.c | 86 ++++++++++++++++++------------------------------------------
2 files changed, 29 insertions(+), 60 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index 59baf62..aefdb28 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -32,13 +32,13 @@
#ifndef WINDOWS32
# include <pwd.h>
+# include <alloca.h>
#endif
#include <errno.h>
#include <dirent.h>
#include <stdlib.h>
#include <string.h>
-#include <alloca.h>
#ifdef _LIBC
# undef strdup
@@ -205,29 +205,6 @@ glob_lstat (glob_t *pglob, int flags, const char *fullname)
: LSTAT64 (fullname, &ust.st64));
}
-/* Set *R = A + B. Return true if the answer is mathematically
- incorrect due to overflow; in this case, *R is the low order
- bits of the correct answer. */
-
-static bool
-size_add_wrapv (size_t a, size_t b, size_t *r)
-{
-#if 5 <= __GNUC__ && !defined __ICC
- return __builtin_add_overflow (a, b, r);
-#else
- *r = a + b;
- return *r < a;
-#endif
-}
-
-static bool
-glob_use_alloca (size_t alloca_used, size_t len)
-{
- size_t size;
- return (!size_add_wrapv (alloca_used, len, &size)
- && __libc_use_alloca (size));
-}
-
static int glob_in_dir (const char *pattern, const char *directory,
int flags, int (*errfunc) (const char *, int),
glob_t *pglob, size_t alloca_used);
@@ -712,11 +689,10 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
else
{
#ifndef WINDOWS32
- char *dirnamestr = char_array_at (&dirname, 0);
- char *end_name = strchr (dirnamestr, '/');
- char *user_name;
- int malloc_user_name = 0;
- char *unescape = NULL;
+ const char *dirnamestr = char_array_str (&dirname);
+ const char *end_name = strchr (dirnamestr, '/');
+ struct char_array user_name;
+ const char *unescape = NULL;
if (!(flags & GLOB_NOESCAPE))
{
@@ -730,27 +706,19 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
unescape = memchr (dirnamestr, '\\', end_name - dirnamestr);
}
if (end_name == NULL)
- user_name = dirnamestr + 1;
+ {
+ if (!char_array_init_str (&user_name, dirnamestr + 1))
+ goto err_nospace;
+ }
else
{
- char *newp;
- if (glob_use_alloca (alloca_used, end_name - dirnamestr))
- newp = alloca_account (end_name - dirnamestr, alloca_used);
- else
- {
- newp = malloc (end_name - dirnamestr);
- if (newp == NULL)
- {
- retval = GLOB_NOSPACE;
- goto out;
- }
- malloc_user_name = 1;
- }
if (unescape != NULL)
{
- char *p = mempcpy (newp, dirnamestr + 1,
- unescape - dirnamestr - 1);
- char *q = unescape;
+ ptrdiff_t name_len = unescape - dirnamestr - 1;
+ if (!char_array_init_str_size (&user_name, dirnamestr + 1,
+ name_len))
+ goto err_nospace;
+ const char *q = unescape;
while (q != end_name)
{
if (*q == '\\')
@@ -761,20 +729,21 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
but "~fo\\o\\/" unescape to user_name
"foo". */
if (filename == NULL)
- *p++ = '\\';
+ char_array_append_char (&user_name, '\\');
break;
}
++q;
}
- *p++ = *q++;
+ char_array_append_char (&user_name, *q++);
}
- *p = '\0';
}
else
- *((char *) mempcpy (newp, dirnamestr + 1,
- end_name - dirnamestr - 1))
- = '\0';
- user_name = newp;
+ {
+ ptrdiff_t name_len = end_name - dirnamestr - 1;
+ if (!char_array_init_str_size (&user_name, dirnamestr + 1,
+ name_len))
+ goto err_nospace;
+ }
}
/* Look up specific user's home directory. */
@@ -786,7 +755,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
# if defined HAVE_GETPWNAM_R || defined _LIBC
struct passwd pwbuf;
- while (getpwnam_r (user_name, &pwbuf,
+ while (getpwnam_r (char_array_str (&user_name), &pwbuf,
pwtmpbuf.data, pwtmpbuf.length, &p)
== ERANGE)
{
@@ -797,11 +766,10 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
}
# else
- p = getpwnam (user_name);
+ p = getpwnam (char_array_str (&user_name));
# endif
- if (__glibc_unlikely (malloc_user_name))
- free (user_name);
+ char_array_free (&user_name);
/* If we found a home directory use this. */
if (p != NULL)
@@ -1024,9 +992,7 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (meta & GLOBPAT_BACKSLASH)
{
char *p = strchr (char_array_str (&dirname), '\\'), *q;
- /* We need to unescape the dirname string. It is certainly
- allocated by alloca, as otherwise filename would be NULL
- or dirname wouldn't contain backslashes. */
+ /* We need to unescape the dirname string. */
q = p;
do
{
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 7/8] posix: Use char_array for home_dir in glob
2017-11-21 13:55 [PATCH 0/8] posix: glob fixes and refactor Adhemerval Zanella
` (6 preceding siblings ...)
2017-11-21 13:55 ` [PATCH 6/8] posix: Remove alloca usage on glob user_name Adhemerval Zanella
@ 2017-11-21 13:55 ` Adhemerval Zanella
2017-11-21 14:24 ` Andreas Schwab
2017-12-12 17:02 ` [PATCH 0/8] posix: glob fixes and refactor Adhemerval Zanella
8 siblings, 1 reply; 15+ messages in thread
From: Adhemerval Zanella @ 2017-11-21 13:55 UTC (permalink / raw)
To: libc-alpha
This patch uses dynarray at glob internal home directory ame
manipulation for GLOB_TILDE. It simplifies it and removes all the
boilerplate buffer managements required.
Checked x86_64-linux-gnu.
* posix/glob.c (__glob): Use char_array for home directory.
Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
ChangeLog | 2 ++
posix/glob.c | 66 ++++++++++++++++++++++++++++++++++++------------------------
2 files changed, 42 insertions(+), 26 deletions(-)
diff --git a/posix/glob.c b/posix/glob.c
index aefdb28..71807d6 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -596,9 +596,14 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
|| char_array_pos (&dirname, 2) == '/')))
{
/* Look up home directory. */
- char *home_dir = getenv ("HOME");
- int malloc_home_dir = 0;
- if (home_dir == NULL || home_dir[0] == '\0')
+ struct char_array home_dir;
+
+ const char *home_env = getenv ("HOME");
+ home_env = home_env == NULL ? "" : home_env;
+ if (!char_array_init_str (&home_dir, home_env))
+ goto err_nospace;
+
+ if (char_array_is_empty (&home_dir))
{
#ifdef WINDOWS32
/* Windows NT defines HOMEDRIVE and HOMEPATH. But give
@@ -608,16 +613,21 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (home_drive != NULL && home_path != NULL)
{
- size_t home_drive_len = strlen (home_drive);
- size_t home_path_len = strlen (home_path);
- char *mem = alloca (home_drive_len + home_path_len + 1);
-
- memcpy (mem, home_drive, home_drive_len);
- memcpy (mem + home_drive_len, home_path, home_path_len + 1);
- home_dir = mem;
+ if (!char_array_set_str (&home_dir, home_drive)
+ || !char_array_append_str (&home_dir, home_path))
+ {
+ char_array_free (&home_dir);
+ goto err_nospace;
+ }
}
else
- home_dir = "c:/users/default"; /* poor default */
+ {
+ if (!char_array_set_str (&home_dir, "c:/users/default"))
+ {
+ char_array_free (&home_dir);
+ goto err_nospace;
+ }
+ }
#else
int err;
struct passwd *p;
@@ -645,44 +655,48 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (!scratch_buffer_grow (&s))
goto err_nospace;
}
- if (err == 0)
- {
- home_dir = strdup (p->pw_dir);
- malloc_home_dir = 1;
- }
+ bool e = (err == 0 && char_array_set_str (&home_dir, p->pw_dir));
scratch_buffer_free (&s);
- if (err == 0 && home_dir == NULL)
+ if (e == false)
goto err_nospace;
#endif /* WINDOWS32 */
}
- if (home_dir == NULL || home_dir[0] == '\0')
+ if (char_array_is_empty (&home_dir))
{
- if (__glibc_unlikely (malloc_home_dir))
- free (home_dir);
if (flags & GLOB_TILDE_CHECK)
{
+ char_array_free (&home_dir);
retval = GLOB_NOMATCH;
goto out;
}
else
{
- home_dir = (char *) "~"; /* No luck. */
- malloc_home_dir = 0;
+ if (!char_array_set_str (&home_dir, "~"))
+ {
+ char_array_free (&home_dir);
+ goto err_nospace;
+ }
}
}
/* Now construct the full directory. */
+ bool e = true;
if (char_array_pos (&dirname, 1) == '\0')
{
- if (!char_array_set_str (&dirname, home_dir))
- goto err_nospace;
+ e = char_array_set_str (&dirname, char_array_str (&home_dir));
dirlen = char_array_size (&dirname) - 1;
}
else
{
/* Replaces '~' by the obtained HOME dir. */
char_array_erase (&dirname, 0);
- if (!char_array_prepend_str (&dirname, home_dir))
- goto err_nospace;
+ e = char_array_prepend_str (&dirname,
+ char_array_str (&home_dir));
+ }
+ if (e == false)
+ {
+ char_array_free (&dirname);
+ char_array_free (&home_dir);
+ goto err_nospace;
}
dirname_modified = true;
}
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 7/8] posix: Use char_array for home_dir in glob
2017-11-21 13:55 ` [PATCH 7/8] posix: Use char_array for home_dir in glob Adhemerval Zanella
@ 2017-11-21 14:24 ` Andreas Schwab
0 siblings, 0 replies; 15+ messages in thread
From: Andreas Schwab @ 2017-11-21 14:24 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
On Nov 21 2017, Adhemerval Zanella <adhemerval.zanella@linaro.org> wrote:
> @@ -645,44 +655,48 @@ __glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
> if (!scratch_buffer_grow (&s))
> goto err_nospace;
> }
> - if (err == 0)
> - {
> - home_dir = strdup (p->pw_dir);
> - malloc_home_dir = 1;
> - }
> + bool e = (err == 0 && char_array_set_str (&home_dir, p->pw_dir));
> scratch_buffer_free (&s);
> - if (err == 0 && home_dir == NULL)
> + if (e == false)
!e
> /* Now construct the full directory. */
> + bool e = true;
> if (char_array_pos (&dirname, 1) == '\0')
> {
> - if (!char_array_set_str (&dirname, home_dir))
> - goto err_nospace;
> + e = char_array_set_str (&dirname, char_array_str (&home_dir));
> dirlen = char_array_size (&dirname) - 1;
> }
> else
> {
> /* Replaces '~' by the obtained HOME dir. */
> char_array_erase (&dirname, 0);
> - if (!char_array_prepend_str (&dirname, home_dir))
> - goto err_nospace;
> + e = char_array_prepend_str (&dirname,
> + char_array_str (&home_dir));
> + }
> + if (e == false)
!e
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/8] posix: glob fixes and refactor
2017-11-21 13:55 [PATCH 0/8] posix: glob fixes and refactor Adhemerval Zanella
` (7 preceding siblings ...)
2017-11-21 13:55 ` [PATCH 7/8] posix: Use char_array for home_dir in glob Adhemerval Zanella
@ 2017-12-12 17:02 ` Adhemerval Zanella
8 siblings, 0 replies; 15+ messages in thread
From: Adhemerval Zanella @ 2017-12-12 17:02 UTC (permalink / raw)
To: libc-alpha
Ping.
On 21/11/2017 11:55, Adhemerval Zanella wrote:
> This patchset main target is to remove alloca usage on glob by
> using a specialized dynarray for C strings (struct char_array).
>
> It does not change any glob semantics, only internal glob buffer
> management. It also does not solve the potential stack overflow
> due recursive allocation from wildcard in patterns [1] (although
> an option would be go to pure dynamic char_array for this case).
>
> [1] http://lists.gnu.org/archive/html/bug-gnulib/2017-10/msg00056.html
>
> Adhemerval Zanella (8):
> malloc: Add specialized dynarray for C strings
> posix: Use char_array for internal glob dirname
> posix: Remove alloca usage for GLOB_BRACE on glob
> posix: Remove alloca usage on glob dirname
> posix: Use dynarray for globname in glob
> posix: Remove alloca usage on glob user_name
> posix: Use char_array for home_dir in glob
> posix: Remove all alloca usage in glob
>
> ChangeLog | 30 ++
> malloc/Makefile | 4 +-
> malloc/Versions | 7 +
> malloc/char_array-impl.c | 57 ++++
> malloc/char_array-skeleton.c | 279 +++++++++++++++++
> malloc/char_array.h | 53 ++++
> malloc/dynarray.h | 9 +
> malloc/dynarray_overflow_failure.c | 31 ++
> malloc/malloc-internal.h | 14 +
> malloc/tst-char_array.c | 112 +++++++
> posix/glob.c | 621 +++++++++++++++----------------------
> 11 files changed, 837 insertions(+), 380 deletions(-)
> create mode 100644 malloc/char_array-impl.c
> create mode 100644 malloc/char_array-skeleton.c
> create mode 100644 malloc/char_array.h
> create mode 100644 malloc/dynarray_overflow_failure.c
> create mode 100644 malloc/tst-char_array.c
>
^ permalink raw reply [flat|nested] 15+ messages in thread