On 3/26/18 4:54 AM, Herbert Xu wrote:
On Mon, Mar 26, 2018 at 03:03:38AM +0200, Harald van Dijk wrote:
This was already the case before your patch, but on systems that flat out
reject paths longer than PATH_MAX (that includes GNU/Linux), it seems weird
that expansion can produce paths which then don't work.
PATH_MAX only applies in certain cases. Besides, you can always
cd into the directories one level at a time to circumvent it.
In other words, when you change the path to a different one than what
globbing produced. That was kind of my point. :)
Besides, even if we did impose a limit here
we'd still have to check that limit at every recursion level which
means that the code won't be that much simpler.
Proof of concept attached. The resulting code is about 100 bytes smaller
at -Os than the dynamic reallocation approach. It's possible to further
reduce that with a statically allocated buffer, but of course that means
a constant memory cost at run time.
Cheers,
Harald van Dijk
--- a/src/expand.c
+++ b/src/expand.c
@@ -122,7 +122,7 @@ STATIC void expandmeta(struct strlist *, int);
#ifdef HAVE_GLOB
STATIC void addglob(const glob_t *);
#else
-STATIC void expmeta(char *, char *);
+STATIC void expmeta(char *);
STATIC struct strlist *expsort(struct strlist *);
STATIC struct strlist *msort(struct strlist *, int);
#endif
@@ -1237,9 +1237,6 @@ addglob(pglob)
#else /* HAVE_GLOB */
-STATIC char *expdir;
-
-
STATIC void
expandmeta(struct strlist *str, int flag)
{
@@ -1261,13 +1258,7 @@ expandmeta(struct strlist *str, int flag)
INTOFF;
p = preglob(str->text, RMESCAPE_ALLOC | RMESCAPE_HEAP);
- {
- int i = strlen(str->text);
- expdir = ckmalloc(i < 2048 ? 2048 : i); /* XXX */
- }
-
- expmeta(expdir, p);
- ckfree(expdir);
+ expmeta(p);
if (p != str->text)
ckfree(p);
INTON;
@@ -1296,7 +1287,7 @@ nometa:
*/
STATIC void
-expmeta(char *enddir, char *name)
+expmeta1(char *expdir, char *enddir, char *name)
{
char *p;
const char *cp;
@@ -1344,6 +1335,8 @@ expmeta(char *enddir, char *name)
metaflag++;
p = name;
do {
+ if (enddir == expdir + PATH_MAX)
+ return;
if (*p == '\\')
p++;
*enddir++ = *p;
@@ -1390,22 +1383,34 @@ expmeta(char *enddir, char *name)
if (dp->d_name[0] == '.' && ! matchdot)
continue;
if (pmatch(start, dp->d_name)) {
+ p = enddir;
+ cp = dp->d_name;
+ for (;;) {
+ if (p == expdir + PATH_MAX)
+ goto toolong;
+ if ((*p++ = *cp++) == '\0')
+ break;
+ }
if (atend) {
- scopy(dp->d_name, enddir);
addfname(expdir);
} else {
- for (p = enddir, cp = dp->d_name;
- (*p++ = *cp++) != '\0';)
- continue;
p[-1] = '/';
- expmeta(p, endname);
+ expmeta1(expdir, p, endname);
}
}
+toolong: ;
}
closedir(dirp);
if (! atend)
endname[-esc - 1] = esc ? '\\' : '/';
}
+
+STATIC void
+expmeta(char *name)
+{
+ char expdir[PATH_MAX];
+ expmeta1(expdir, expdir, name);
+}
#endif /* HAVE_GLOB */
