Good morning.
On Sun, Nov 10, 2019 at 6:15 PM Paul Eggert <egg...@cs.ucla.edu> wrote:
> It seems to me that the performance issues are still there, in the sense that
> if
> GLOB_ONLYDIR is set then 'glob' now does a stat call on each directory entry
> to
> see whether it's a directory.
The idea is that this patch relieves the program from having to call
stat and the total number of stats stays the same.
However, since you don't like the idea here is a patch that only fixes
trailing slash.
regards, Dmitry
diff --git a/lib/glob.c b/lib/glob.c
index 80233fdec..2b5195572 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -1113,10 +1113,11 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
if (flags & GLOB_MARK)
{
- /* Append slashes to directory names. */
- size_t i;
+ /* Append slashes to directory names.
+ If GLOB_ONLYDIR is set filter out files and symlinks to files. */
+ size_t i, e;
- for (i = oldcount; i < pglob->gl_pathc + pglob->gl_offs; ++i)
+ for (i = e = oldcount; i < pglob->gl_pathc + pglob->gl_offs; ++i)
if (is_dir (pglob->gl_pathv[i], flags, pglob))
{
size_t len = strlen (pglob->gl_pathv[i]) + 2;
@@ -1129,8 +1130,46 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
goto out;
}
strcpy (&new[len - 2], "/");
+ if (pglob->gl_pathv[e] == NULL)
+ {
+ pglob->gl_pathv[e++] = new;
+ pglob->gl_pathv[i] = NULL;
+ }
+ else
pglob->gl_pathv[i] = new;
+
+ }
+ else if (flags & GLOB_ONLYDIR)
+ {
+ free (pglob->gl_pathv[i]);
+ pglob->gl_pathv[i] = NULL;
+ if (pglob->gl_pathv[e] != NULL)
+ e = i;
+ }
+ if (pglob->gl_pathv[e] == NULL)
+ pglob->gl_pathc = e - pglob->gl_offs;
+ if ((flags & GLOB_NOCHECK) && pglob->gl_pathc <= oldcount)
+ {
+ size_t len = strlen (pattern) + 2;
+ char *new = malloc (len);
+ if (new == NULL)
+ {
+ globfree (pglob);
+ pglob->gl_pathc = 0;
+ retval = GLOB_NOSPACE;
+ goto out;
}
+ memcpy (new, pattern, len - 2);
+ new[len - 2] = '/';
+ new[len - 1] = '\0';
+ pglob->gl_pathv[oldcount] = new;
+ if (flags & GLOB_APPEND)
+ pglob->gl_pathc = oldcount - pglob->gl_offs + 1;
+ else
+ pglob->gl_pathc = 1;
+ }
+ else if (pglob->gl_pathc == 0)
+ retval = GLOB_NOMATCH;
}
if (!(flags & GLOB_NOSORT))
diff --git a/tests/test-glob.c b/tests/test-glob.c
index 74cde45c2..3deee5ac9 100644
--- a/tests/test-glob.c
+++ b/tests/test-glob.c
@@ -28,17 +28,178 @@ SIGNATURE_CHECK (globfree, void, (glob_t *));
#include <errno.h>
#include <string.h>
#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <stdarg.h>
+#include <stdbool.h>
#include "macros.h"
+static void
+touch (const char *p)
+{
+ int fd = open (p, O_CREAT, S_IRUSR | S_IWUSR);
+ ASSERT (fd > 0);
+ close (fd);
+}
+
+static void
+md (const char *p)
+{
+ int rc = mkdir (p, 0770);
+ ASSERT (rc == 0);
+}
+
+static void
+ln (const char *old, const char *new)
+{
+ int rc = link (old, new);
+ ASSERT (rc == 0);
+}
+
+static void
+ln_s (const char *old, const char *new)
+{
+ int rc = symlink (old, new);
+ ASSERT (rc == 0);
+}
+
+static const char *
+flags2str (int flags)
+{
+ if (flags == 0)
+ return "0";
+
+ static const char mark[] = "GLOB_MARK";
+ static const char dooffs[] = "GLOB_DOOFFS";
+ static const char nocheck[] = "GLOB_NOCHECK";
+ static const char append[] = "GLOB_APPEND";
+ static const char brace[] = "GLOB_BRACE";
+ static const char onlydir[] = "GLOB_ONLYDIR";
+ static const char p[] = "|";
+ const size_t nmark = sizeof (mark) - 1;
+ const size_t ndooffs = sizeof (dooffs) - 1;
+ const size_t nnocheck = sizeof (nocheck) - 1;
+ const size_t nappend = sizeof (append) - 1;
+ const size_t nbrace = sizeof (brace) - 1;
+ const size_t nonlydir = sizeof (onlydir) - 1;
+
+ static char buf[1024]; /* No need to memset buf. */
+ char *n = buf;
+ const char *d = "";
+ if (flags & GLOB_MARK)
+ n = mempcpy (n, mark, nmark), d = p;
+ if (flags & GLOB_DOOFFS)
+ n = mempcpy (mempcpy (n, d, strlen (d)), dooffs, ndooffs), d = p;
+ if (flags & GLOB_NOCHECK)
+ n = mempcpy (mempcpy(n, d, strlen (d)), nocheck, nnocheck), d = p;
+ if (flags & GLOB_APPEND)
+ n = mempcpy (mempcpy(n, d, strlen (d)), append, nappend), d = p;
+ if (flags & GLOB_BRACE)
+ n = mempcpy (mempcpy(n, d, strlen (d)), brace, nbrace), d = p;
+ if (flags & GLOB_ONLYDIR)
+ n = mempcpy (mempcpy(n, d, strlen (d)), onlydir, nonlydir), d = p;
+ *n = '\0';
+ return buf;
+}
+
+static bool verbose = false;
+
+static void
+cmp (size_t offs, size_t oldpathc, const glob_t *g, va_list ap)
+{
+ size_t k = oldpathc;
+ for (;;)
+ {
+ const char *s = va_arg (ap, const char *);
+ if (s == NULL)
+ break;
+ ASSERT (k < g->gl_pathc);
+ if (verbose)
+ printf ("%s\n", s);
+ int rc = strcmp (g->gl_pathv[k + offs], s);
+ ASSERT (rc == 0);
+ ++k;
+ }
+ ASSERT (g->gl_pathc == k);
+}
+
+static void
+testimp (int rc, int flags, size_t offs, const char *pattern, va_list ap)
+{
+ int res;
+ glob_t g;
+ va_list ap2;
+ va_copy (ap2, ap);
+
+ if (verbose)
+ printf ("pattern=%s, flags=0x%x (%s), offs=%zu\n", pattern,
+ flags, flags2str (flags), offs);
+ memset (&g, 0, sizeof g);
+ g.gl_offs = offs;
+ res = glob (pattern, flags, NULL, &g);
+ ASSERT (res == rc);
+ cmp (offs, 0, &g, ap);
+ if (rc != 0)
+ return;
+
+ flags |= GLOB_APPEND;
+ if (verbose)
+ printf ("pattern=%s, flags=0x%x (%s), offs=%zu\n", pattern,
+ flags, flags2str (flags), offs);
+ size_t pathc = g.gl_pathc;
+ res = glob (pattern, flags, NULL, &g);
+ ASSERT (res == rc);
+ cmp (offs, pathc, &g, ap2);
+
+ globfree (&g);
+}
+
+static void
+test (int rc, int flags, const char *pattern, ...)
+{
+ va_list ap;
+
+ va_start (ap, pattern);
+ testimp (rc, flags, 0, pattern, ap);
+ va_end (ap);
+
+ va_start (ap, pattern);
+ testimp (rc, flags | GLOB_DOOFFS, 2, pattern, ap);
+ va_end (ap);
+
+ if (rc == GLOB_NOMATCH)
+ test (0, flags | GLOB_NOCHECK, pattern, pattern, NULL);
+}
+
+static void
+cleanup ()
+{
+ unlink ("hellod/worldd/kenf1");
+ unlink ("hellod/worldd/kenf2");
+ unlink ("hellod/worldf");
+ unlink ("hellofbs");
+ unlink ("hellofs");
+ unlink ("hellofl");
+ unlink ("hellof");
+ rmdir ("hellod/worldd/kend1");
+ rmdir ("hellod/worldd/kend2");
+ rmdir ("hellod/worldd");
+ unlink ("hellods");
+ rmdir ("hellod");
+}
+
#define BASE "test-glob.t"
#define GL_NO_SUCH_FILE "/gnulib-magic-does-not-exist"
int
-main ()
+main (int argc, char *argv[])
{
int res;
glob_t g;
+ verbose = argc > 1;
+ (void) argv;
res = glob (".", 0, NULL, &g);
ASSERT (res == 0 && g.gl_pathc == 1);
@@ -86,6 +247,152 @@ main ()
ASSERT (strcmp (g.gl_pathv[1], BASE "globlink2/") == 0);
globfree (&g);
}
+ unlink (BASE "globlink1");
+ unlink (BASE "globlink2");
+
+ /* Test that / matches explicitly. */
+ cleanup ();
+
+ test (GLOB_NOMATCH, 0, "", NULL);
+ test (GLOB_NOMATCH, 0, "hello", NULL);
+ test (GLOB_NOMATCH, 0, "hello*", NULL);
+ test (GLOB_NOMATCH, 0, "hello*/", NULL);
+
+ touch ("hellof");
+ test (0, 0, "hello*", "hellof", NULL);
+ test (GLOB_NOMATCH, 0, "hello*/", NULL);
+
+ md ("hellod");
+ ln_s ("hellod", "hellods");
+ ln_s ("broken_symlink", "hellofbs");
+ ln ("hellof", "hellofl");
+ ln_s ("hellof", "hellofs");
+ touch ("hellod/worldf");
+ md ("hellod/worldd");
+
+ test (0, 0, "hello*", "hellod", "hellods", "hellof", "hellofbs",
+ "hellofl", "hellofs", NULL);
+ test (0, GLOB_MARK, "hello*", "hellod/", "hellods/", "hellof",
+ "hellofbs", "hellofl", "hellofs", NULL);
+ test (0, GLOB_ONLYDIR | GLOB_MARK, "hello*", "hellod/", "hellods/", NULL);
+
+ test (0, 0, "hello*/", "hellod/", "hellods/", NULL);
+
+ test (0, GLOB_MARK, "hello*/", "hellod/", "hellods/", NULL);
+
+ test (0, GLOB_ONLYDIR, "hello*/", "hellod/", "hellods/", NULL);
+ test (0, GLOB_MARK | GLOB_ONLYDIR, "hello*/", "hellod/", "hellods/", NULL);
+
+ test (0, 0, "hellod/world*", "hellod/worldd", "hellod/worldf", NULL);
+ test (0, GLOB_MARK, "hellod/world*", "hellod/worldd/", "hellod/worldf",
+ NULL);
+ test (0, GLOB_MARK | GLOB_ONLYDIR, "hellod/world*", "hellod/worldd/", NULL);
+
+ test (0, 0, "hello*/world*", "hellod/worldd", "hellod/worldf",
+ "hellods/worldd", "hellods/worldf", NULL);
+ test (0, GLOB_MARK, "hello*/world*", "hellod/worldd/", "hellod/worldf",
+ "hellods/worldd/", "hellods/worldf", NULL);
+
+ test (0, GLOB_MARK | GLOB_ONLYDIR, "hello*/world*", "hellod/worldd/",
+ "hellods/worldd/", NULL);
+
+ test (0, 0, "hellod/world*/", "hellod/worldd/", NULL);
+ test (0, GLOB_MARK, "hellod/world*/", "hellod/worldd/", NULL);
+ test (0, GLOB_ONLYDIR, "hellod/world*/", "hellod/worldd/", NULL);
+ test (0, GLOB_MARK | GLOB_ONLYDIR, "hellod/world*/", "hellod/worldd/", NULL);
+
+ test (0, 0, "hellod/*", "hellod/worldd", "hellod/worldf", NULL);
+ test (0, GLOB_MARK, "hellod/*", "hellod/worldd/", "hellod/worldf", NULL);
+
+ test (0, GLOB_MARK | GLOB_ONLYDIR, "hellod/*", "hellod/worldd/", NULL);
+
+ test (0, 0, "hellod/*/", "hellod/worldd/", NULL);
+ test (0, GLOB_MARK, "hellod/*/", "hellod/worldd/", NULL);
+ test (0, GLOB_ONLYDIR, "hellod/*/", "hellod/worldd/", NULL);
+ test (0, GLOB_MARK | GLOB_ONLYDIR, "hellod/*/", "hellod/worldd/", NULL);
+
+ test (0, 0, "*/world*", "hellod/worldd", "hellod/worldf", "hellods/worldd",
+ "hellods/worldf", NULL);
+ test (0, GLOB_MARK, "*/world*", "hellod/worldd/", "hellod/worldf",
+ "hellods/worldd/", "hellods/worldf", NULL);
+ test (0, GLOB_MARK | GLOB_ONLYDIR, "*/world*", "hellod/worldd/",
+ "hellods/worldd/", NULL);
+
+ test (0, 0, "*/world*/", "hellod/worldd/", "hellods/worldd/", NULL);
+ test (0, GLOB_MARK, "*/world*/", "hellod/worldd/", "hellods/worldd/", NULL);
+ test (0, GLOB_ONLYDIR, "*/world*/", "hellod/worldd/", "hellods/worldd/",
+ NULL);
+ test (0, GLOB_MARK | GLOB_ONLYDIR, "*/world*/", "hellod/worldd/",
+ "hellods/worldd/", NULL);
+
+
+ test (0, GLOB_BRACE, "hellod/{,worldd,worldf}", "hellod/", "hellod/worldd",
+ "hellod/worldf", NULL);
+ test (0, GLOB_BRACE, "{hellod/{,worldd,worldf},hellof}", "hellod/",
+ "hellod/worldd", "hellod/worldf", "hellof", NULL);
+ test (0, GLOB_BRACE, "{hello*/{,world*},hellof}", "hellod/",
+ "hellods/", "hellod/worldd", "hellod/worldf", "hellods/worldd",
+ "hellods/worldf", "hellof", NULL);
+ test (0, GLOB_MARK | GLOB_BRACE, "{hello*/{,world*},hellof}",
+ "hellod/", "hellods/", "hellod/worldd/", "hellod/worldf",
+ "hellods/worldd/", "hellods/worldf", "hellof", NULL);
+ test (0, GLOB_MARK | GLOB_ONLYDIR | GLOB_BRACE,
+ "{hello*/{,world*},hellof}", "hellod/", "hellods/", "hellod/worldd/",
+ "hellods/worldd/", NULL);
+ test (0, GLOB_BRACE, "{hello*/{,world*/},hellof}", "hellod/",
+ "hellods/", "hellod/worldd/", "hellods/worldd/", "hellof", NULL);
+ test (0, GLOB_MARK | GLOB_BRACE, "{hello*/{,world*/},hellof}",
+ "hellod/", "hellods/", "hellod/worldd/", "hellods/worldd/", "hellof",
+ NULL);
+ test (0, GLOB_MARK | GLOB_ONLYDIR | GLOB_BRACE,
+ "{hello*/{,world*/},hellof}", "hellod/", "hellods/",
+ "hellod/worldd/", "hellods/worldd/", NULL);
+
+ md ("hellod/worldd/kend1");
+ md ("hellod/worldd/kend2");
+ touch ("hellod/worldd/kenf1");
+ touch ("hellod/worldd/kenf2");
+
+ test (0, 0, "hellod/*/ken*", "hellod/worldd/kend1",
+ "hellod/worldd/kend2", "hellod/worldd/kenf1", "hellod/worldd/kenf2",
+ NULL);
+ test (0, GLOB_MARK, "hellod/*/ken*", "hellod/worldd/kend1/",
+ "hellod/worldd/kend2/", "hellod/worldd/kenf1", "hellod/worldd/kenf2",
+ NULL);
+ test (0, GLOB_MARK | GLOB_ONLYDIR, "hellod/*/ken*",
+ "hellod/worldd/kend1/", "hellod/worldd/kend2/", NULL);
+
+
+ test (0, 0, "hellod/*/ken*/", "hellod/worldd/kend1/", "hellod/worldd/kend2/",
+ NULL);
+ test (0, GLOB_MARK, "hellod/*/ken*/", "hellod/worldd/kend1/",
+ "hellod/worldd/kend2/", NULL);
+ test (0, GLOB_ONLYDIR, "hellod/*/ken*/", "hellod/worldd/kend1/",
+ "hellod/worldd/kend2/", NULL);
+ test (0, GLOB_MARK | GLOB_ONLYDIR, "hellod/*/ken*/",
+ "hellod/worldd/kend1/", "hellod/worldd/kend2/", NULL);
+
+ test (0, 0, "hellod/*/ken?[12]", "hellod/worldd/kend1",
+ "hellod/worldd/kend2", "hellod/worldd/kenf1", "hellod/worldd/kenf2",
+ NULL);
+ test (0, GLOB_MARK, "hellod/*/ken?[12]", "hellod/worldd/kend1/",
+ "hellod/worldd/kend2/", "hellod/worldd/kenf1", "hellod/worldd/kenf2",
+ NULL);
+ test (0, GLOB_MARK | GLOB_ONLYDIR, "hellod/*/ken?[12]",
+ "hellod/worldd/kend1/", "hellod/worldd/kend2/", NULL);
+
+
+ test (0, 0, "hellod/*/ken?[12]/", "hellod/worldd/kend1/",
+ "hellod/worldd/kend2/", NULL);
+ test (0, GLOB_MARK, "hellod/*/ken?[12]/", "hellod/worldd/kend1/",
+ "hellod/worldd/kend2/", NULL);
+ test (0, GLOB_ONLYDIR, "hellod/*/ken?[12]/",
+ "hellod/worldd/kend1/", "hellod/worldd/kend2/", NULL);
+ test (0, GLOB_MARK | GLOB_ONLYDIR, "hellod/*/ken?[12]/",
+ "hellod/worldd/kend1/", "hellod/worldd/kend2/", NULL);
+
+
+ cleanup ();
return 0;
}
