Date: Wed, 9 Nov 2011 13:09:22 +0800 (CST) From: Cheng-Lung Sung <clsung@FreeBSD.org> To: FreeBSD-gnats-submit@FreeBSD.org Subject: misc/162396: [patch] remove loop in globpexp1()@lib/libc/gen/glob.c Message-ID: <20111109050922.2671A1148E@going04.iis.sinica.edu.tw> Resent-Message-ID: <201111090510.pA95A84M051385@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 162396
>Category: misc
>Synopsis: [patch] remove loop in globpexp1()@lib/libc/gen/glob.c
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: change-request
>Submitter-Id: current-users
>Arrival-Date: Wed Nov 09 05:10:08 UTC 2011
>Closed-Date:
>Last-Modified:
>Originator: Cheng-Lung Sung
>Release: FreeBSD 8.2-RELEASE amd64
>Organization:
FreeBSD @ Taiwan
>Environment:
System: FreeBSD going04.iis.sinica.edu.tw 8.2-RELEASE FreeBSD 8.2-RELEASE #0: Tue Mar 8 12:09:34 CST 2011 root@going04.iis.sinica.edu.tw:/usr/obj/amd64/usr/src/sys/GOING04_SMP amd64
>Description:
One can see that globexp1() is called with GLOB_BRACE.
In globexp1(), examine the pattern:
only "{}" => call glob0()
no LBRACE => call glob0()
with LBRACE => call globexp2 to deal with expression inside LBRACE
Take a look in glob0():
glob0, return 0 if success, otherwise error (except GLOB_NOMATCH)
=> return err
=> return globextend()/GLOB_NOMATCH
=> return 0
In globexp2()
=> non matched braces
=> call glob0(), return 0
=> matched brace-pair
=> parse brace
=> reach rbrace, call globexp1(), set return value to *rv
set *rv = 0, return 0
*which implies always return 0 in globexp2*
As a result, the following code block in globexp1 can be reduced from:
- while ((ptr = g_strchr(ptr, LBRACE)) != NULL)
- if (!globexp2(ptr, pattern, pglob, &rv, limit))
- return rv;
to:
+ if ((ptr = g_strchr(ptr, LBRACE)) != NULL)
+ return globexp2(ptr, pattern, pglob, limit)
Besides, since *rv should indicate the acctual return value, the setting *rv = 0 and return 0 is ambiguous.
So the flow in globexp2() should be:
In globexp2()
=> non matched braces
=> call glob0(), return 0
=> matched brace-pair
=> parse brace
=> reach rbrace, call globexp1(), set return value to *rv
=> if success in glob0
return 0
=> if failed in glob0 (but not GLOB_NOMATCH)
return err
>How-To-Repeat:
>Fix:
diff --git a/lib/libc/gen/glob.c b/lib/libc/gen/glob.c
index c3d9f08..40c5808 100644
--- a/lib/libc/gen/glob.c
+++ b/lib/libc/gen/glob.c
@@ -156,7 +156,7 @@ static int globextend(const Char *, glob_t *, size_t *);
static const Char *
globtilde(const Char *, Char *, size_t, glob_t *);
static int globexp1(const Char *, glob_t *, size_t *);
-static int globexp2(const Char *, const Char *, glob_t *, int *, size_t *);
+static int globexp2(const Char *, const Char *, glob_t *, size_t *);
static int match(Char *, Char *, Char *);
#ifdef DEBUG
static void qprintf(const char *, Char *);
@@ -240,15 +240,13 @@ static int
globexp1(const Char *pattern, glob_t *pglob, size_t *limit)
{
const Char* ptr = pattern;
- int rv;
/* Protect a single {}, for find(1), like csh */
if (pattern[0] == LBRACE && pattern[1] == RBRACE && pattern[2] == EOS)
return glob0(pattern, pglob, limit);
- while ((ptr = g_strchr(ptr, LBRACE)) != NULL)
- if (!globexp2(ptr, pattern, pglob, &rv, limit))
- return rv;
+ if ((ptr = g_strchr(ptr, LBRACE)) != NULL)
+ return globexp2(ptr, pattern, pglob, limit)
return glob0(pattern, pglob, limit);
}
@@ -260,9 +258,10 @@ globexp1(const Char *pattern, glob_t *pglob, size_t *limit)
* If it fails then it tries to glob the rest of the pattern and returns.
*/
static int
-globexp2(const Char *ptr, const Char *pattern, glob_t *pglob, int *rv, size_t *limit)
+globexp2(const Char *ptr, const Char *pattern, glob_t *pglob, size_t *limit)
{
int i;
+ int rv;
Char *lm, *ls;
const Char *pe, *pm, *pm1, *pl;
Char patbuf[MAXPATHLEN];
@@ -297,8 +296,7 @@ globexp2(const Char *ptr, const Char *pattern, glob_t *pglob, int *rv, size_t *l
/* Non matching braces; just glob the pattern */
if (i != 0 || *pe == EOS) {
- *rv = glob0(patbuf, pglob, limit);
- return 0;
+ return glob0(patbuf, pglob, limit);
}
for (i = 0, pl = pm = ptr; pm <= pe; pm++)
@@ -344,7 +342,9 @@ globexp2(const Char *ptr, const Char *pattern, glob_t *pglob, int *rv, size_t *l
#ifdef DEBUG
qprintf("globexp2:", patbuf);
#endif
- *rv = globexp1(patbuf, pglob, limit);
+ rv = globexp1(patbuf, pglob, limit);
+ if (rv && rv != GLOB_NOMATCH) /* if errors occured */
+ return rv;
/* move after the comma, to the next string */
pl = pm + 1;
@@ -354,7 +354,6 @@ globexp2(const Char *ptr, const Char *pattern, glob_t *pglob, int *rv, size_t *l
default:
break;
}
- *rv = 0;
return 0;
}
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20111109050922.2671A1148E>