[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/8] posix: Use char_array for internal glob dirname
|
From: |
Arjun Shankar |
|
Subject: |
Re: [PATCH 2/8] posix: Use char_array for internal glob dirname |
|
Date: |
Tue, 23 Mar 2021 16:08:51 +0000 |
|
User-agent: |
Mutt/1.10.1 (2018-07-13) |
Hi Adhemerval,
> 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.
I've been going through this patch series. Here are my comments on the
first one. I have mostly been looking at this from the point of view of
making sure the logic remains equivalent, and not the way you tackled
the problem itself.
In summary, I found a comparison reversed, a missed concatenation, and
some minor indent issues. Other than that, this patch looks good to me.
Details:
> diff --git a/posix/glob.c b/posix/glob.c
> index 32c88e5d15..8c6e1914c5 100644
> --- a/posix/glob.c
> +++ b/posix/glob.c
> @@ -85,6 +85,7 @@
> #include <flexmember.h>
> #include <glob_internal.h>
> #include <scratch_buffer.h>
> +#include <malloc/char_array-skeleton.c>
Include the new dynamic character array implementation. OK.
Note:
The patch that introduces char_array-skeleton.c will need a slight adjustment
after de0e1b45b0ab (malloc: Sync dynarray with gnulib) due to the removal
of the anonymous union.
> static const char *next_brace_sub (const char *begin, int flags) __THROWNL;
>
> @@ -298,16 +299,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;
> glob_t dirs;
> int retval = 0;
> size_t alloca_used = 0;
> + struct char_array dirname;
> + bool dirname_modified;
dirname changes type, dirname_modified should be a boolean, and malloc_dirname
is now redundant.
OK.
> if (pattern == NULL || pglob == NULL || (flags & ~__GLOB_FLAGS) != 0)
> {
> @@ -315,6 +315,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] == '/')
OK.
> @@ -335,12 +339,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 *));
err_nospace frees dirname and returns GLOB_NOSPACE.
So the code is equivalent.
OK.
> if (pglob->gl_pathv == NULL)
> - return GLOB_NOSPACE;
> + goto err_nospace;
>
> for (i = 0; i <= pglob->gl_offs; ++i)
> pglob->gl_pathv[i] = NULL;
> @@ -392,7 +396,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. */
OK. Same.
> @@ -454,7 +458,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 == '}')
out frees dirname and returns retval.
So the code is equivalent.
OK.
> @@ -471,9 +476,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;
We will continue at 'out', which will also return after freeing dirname.
So the code remains equivalent.
OK.
> else if (!(flags & (GLOB_NOCHECK|GLOB_NOMAGIC)))
> - return GLOB_NOMATCH;
> + retval = GLOB_NOMATCH;
> + goto out;
> }
> }
>
OK. Same here.
> @@ -492,14 +498,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)
> {
OK. It's declared as a boolean now.
> /* 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
OK. It's still equivalent. Since char_array_set_str can lead to a failed
allocation, we add a check and exit with error if that happens.
Indent is a bit off, though.
> @@ -516,7 +523,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;
> }
> }
OK. Same as last. Return an error if the allocation fails.
Indent is a bit off.
> @@ -525,13 +533,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;
> }
OK.
> else
> {
> - char *newp;
> dirlen = filename - pattern;
> #if defined __MSDOS__ || defined WINDOWS32
> if (*filename == ':'
OK. newp was used for malloc/alloca allocations.
> @@ -545,31 +553,26 @@ __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;
> + }
We need to deallocate before every return now. This does that.
OK.
> /* 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;
We used to alloca/malloc some space and copy pattern into it. We still do
the same but using a dynarray. So don't need malloc_dirname any more.
OK.
> #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
Looks like the comparisons have been reversed. I think they should be `=='
instead of `!='.
> @@ -578,20 +581,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) == '\\')
> {
OK.
> /* "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);
> }
> }
p was a pointer to the last character in dirname, and we looped on it going
backwards towards the start of dirname looking for a '\'.
Now, p is an index into dirname and we do the same thing.
Looks equivalent, and more readable.
OK.
> - 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));
OK.
> @@ -608,11 +615,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) == '~')
> {
OK.
> - 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) == '/')))
OK. They do the same thing.
Indent needs a fix.
> {
> /* Look up home directory. */
> char *home_dir = getenv ("HOME");
> @@ -663,10 +673,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)
> {
OK.
> @@ -675,10 +682,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')
OK.
> @@ -697,53 +701,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')
> {
OK.
> - if (__glibc_unlikely (malloc_dirname))
> - free (dirname);
> -
OK. We don't use malloc for dirname any more.
> - 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;
Equivalent, and we don't use malloc_dirname any more so we throw away that
assignment.
OK.
> }
> 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;
> - }
> - }
This code was allocating enough memory to concatenate home_dir and dirname.
> - mempcpy (mempcpy (newp, home_dir, home_len),
> - &dirname[1], dirlen);
...Then concatenating it.
> - 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);
...And freeing any previously allocated memory.
> + /* Replaces '~' by the obtained HOME dir. */
> + char_array_erase (&dirname, 0);
> + if (!char_array_prepend_str (&dirname, home_dir))
> + goto err_nospace;
Now we do it using a dynarray.
OK. Indent needs a fix.
> }
> - dirname_modified = 1;
> + dirname_modified = true;
> }
OK.
> else
> {
> #ifndef WINDOWS32
> - char *end_name = strchr (dirname, '/');
> + char *dirnamestr = char_array_at (&dirname, 0);
> + char *end_name = strchr (dirnamestr, '/');
Equivalent. OK.
> char *user_name;
> int malloc_user_name = 0;
> char *unescape = NULL;
> @@ -752,23 +729,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');
> }
OK. Indent needs a fix, and further down as well.
> else
> - unescape = memchr (dirname, '\\', end_name - dirname);
> + unescape = memchr (dirnamestr, '\\', end_name - dirnamestr);
> }
OK.
> if (end_name == NULL)
> - user_name = dirname + 1;
> + user_name = dirnamestr + 1;
OK.
> 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;
OK. This newp is for user_name which is tackled in a separate patch.
> @@ -778,8 +755,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)
> {
> @@ -801,8 +778,9 @@ __glob (const char *pattern, int flags, int (*errfunc)
> (const char *, int),
> *p = '\0';
> }
> else
> - *((char *) mempcpy (newp, dirname + 1, end_name - dirname -
> 1))
> - = '\0';
> + *((char *) mempcpy (newp, dirnamestr + 1,
> + end_name - dirnamestr - 1))
> + = '\0';
> user_name = newp;
> }
Same. OK. There appears to be a changed line due to a stray whitespace.
> @@ -835,39 +813,14 @@ __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);
> - /* dirname contains end_name; we can't free it now. */
> - char *prev_dirname =
> - (__glibc_unlikely (malloc_dirname) ? dirname : NULL);
> - char *d;
> -
> - 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)
> - {
> - free (prev_dirname);
> - scratch_buffer_free (&pwtmpbuf);
> - retval = GLOB_NOSPACE;
> - goto out;
> - }
> - malloc_dirname = 1;
> + scratch_buffer_free (&pwtmpbuf);
> + goto err_nospace;
> }
> - d = mempcpy (dirname, p->pw_dir, home_len);
> - if (end_name != NULL)
> - d = mempcpy (d, end_name, rest_len);
> - *d = '\0';
> -
> - free (prev_dirname);
>
> - dirlen = home_len + rest_len;
> - dirname_modified = 1;
> + dirlen = strlen (p->pw_dir);
> + dirname_modified = true;
> }
It appears that a concatenation (p->pw_dir and end_name) got missed here.
> else
> {
> @@ -908,37 +861,33 @@ __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))
OK.
> {
> 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);
OK.
> p[0] = '/';
> p[1] = '\0';
> - if (__glibc_unlikely (malloc_dirname))
> - free (dirname);
> }
OK.
> 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;
> }
OK.
> pglob->gl_pathv[++newcount] = NULL;
> ++pglob->gl_pathc;
> pglob->gl_flags = flags;
>
> - return 0;
> + goto out;
> }
OK. 'out' so we can deallocate before returning.
>
> - meta = __glob_pattern_type (dirname, !(flags & GLOB_NOESCAPE));
> + meta = __glob_pattern_type (char_array_str (&dirname),
> + !(flags & GLOB_NOESCAPE));
OK.
> /* 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
> @@ -951,15 +900,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) == '\\')
OK.
> {
> /* "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);
Equivalent. We use an index instead of a pointer, and step back from the
end. OK.
> }
>
> if (__glibc_unlikely ((flags & GLOB_ALTDIRFUNC) != 0))
> @@ -973,7 +922,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);
OK.
> @@ -1020,8 +969,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;
OK.
> }
> }
>
> @@ -1043,8 +991,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,
> @@ -1059,8 +1006,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;
> }
Same.
>
> ++pglob->gl_pathc;
> @@ -1086,7 +1032,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;
Okay.
> /* 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. */
> @@ -1103,12 +1049,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);
OK.
> if (status != 0)
> {
> if (status == GLOB_NOMATCH && flags != orig_flags
> @@ -1126,14 +1072,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))
OK.
> {
> globfree (pglob);
> pglob->gl_pathc = 0;
> - retval = GLOB_NOSPACE;
> - goto out;
> + goto err_nospace;
OK.
> }
> }
> }
> @@ -1152,8 +1097,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;
Same.
> }
> strcpy (&new[len - 2], "/");
> pglob->gl_pathv[i] = new;
> @@ -1169,10 +1113,13 @@ __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;
> }
OK. out and err_nospace, both deallocate before returning.
Cheers,
Arjun
- Re: [PATCH 2/8] posix: Use char_array for internal glob dirname,
Arjun Shankar <=