From patchwork Tue Nov 21 13:55:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 24404 Received: (qmail 44392 invoked by alias); 21 Nov 2017 13:55:47 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 44165 invoked by uid 89); 21 Nov 2017 13:55:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KB_WAM_FROM_NAME_SINGLEWORD, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=Stick, 9927, 11348 X-HELO: mail-qk0-f196.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=e3M99bh8PdxKe4J/u9m76Lg406TnBBpuz7mY/QGwLFU=; b=EhyyfCfPl8PYCmZOxE0l0GLk+vUKq27epbkAx7ujQ0IivuIdW6Xza7utEjZlPs5UJk nDcksD61baIy8+WH8D9NRRx1Lp0KnbOmXJDkr86k7o9+Bl0BsTkszQg/6+YV6l07w63a iSoLb48pPL+Io1Ut8QaZMY7kdQm3hxLEWLb4zjmFighImfuuS3Ix++TgsKjHoTSAkCwN sxmEMW/q8DwI9BCdawypAhpKZBmmXm8l6shyE4SPTekbvODdDh/e3kptzKxDfUj26jqC SYUcF3G5XhjU2SpPz+JpIiF0qN/ReMrCuhOag/83bl9JX5OdpPquWr5Y5xVvx+SZ9SVk fgzg== X-Gm-Message-State: AJaThX71rFpRyQXUzcl0y/LW0Emo6EaYF0IdcLDVN0i00++oJ2aKSpMY PbQoWiveobpMsjPRoFcuB8L9Zfp7R4Y= X-Google-Smtp-Source: AGs4zMbZLad4GlIhKWfcA4dquvDz3Bu//jgNjvK7sHiEjaLr4N8MWq4smMSVP/IeFnNMQZEi4K3eCA== X-Received: by 10.55.8.4 with SMTP id 4mr6803771qki.249.1511272540472; Tue, 21 Nov 2017 05:55:40 -0800 (PST) From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH 2/8] posix: Use char_array for internal glob dirname Date: Tue, 21 Nov 2017 11:55:24 -0200 Message-Id: <1511272530-10936-3-git-send-email-adhemerval.zanella@linaro.org> In-Reply-To: <1511272530-10936-1-git-send-email-adhemerval.zanella@linaro.org> References: <1511272530-10936-1-git-send-email-adhemerval.zanella@linaro.org> 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 --- 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 #include #include +#include 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);