Good morning.
On Sat, Nov 9, 2019 at 8:13 PM Bruno Haible <br...@clisp.org> wrote:
> Are these issues supposed to be fixed in the new patch?
yes.
> (I appreciate that you add tests!):
It is easier to write a test program than to test glob manually.
> - Please follow GNU coding style, esp. regarding space between a function
> name and its arguments:
> int rc = symlink (old, new);
Done.
> - Similarly, add spaces around operators:
> GLOB_MARK | GLOB_ONLYDIR
Done.
> - The functions touch, md, ln, ln_s all return 0; better change the return
> type to 'void'.
Done
> - Why is 'verbose' an 'int'?
Personal preference.
> Couldn't it be a 'bool'?
Changed to bool.
1. To reproduce that a pattern with a trailing slash fails to filter
out files you'll libc with dirent that has no type field. That could
be an old glibc or sunos or aix libc.
2. You can use any libc to reproduce that GLOB_ONLYDIR fails to filter
out symlinks to files.
Thanks for review.
regards, Dmitry
diff --git a/lib/glob.c b/lib/glob.c
index 80233fdec..ef0c83625 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -1111,16 +1111,22 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
}
}
- if (flags & GLOB_MARK)
+ /* The following loop filters out files and symlinks to files.
+ This is needed when readdir_result_type returns DT_UNKNOWN and DT_LNK.
+ If GLOB_MARK is set the loop also marks directories. */
+ if ((flags & GLOB_MARK) || (flags & GLOB_ONLYDIR))
{
/* Append slashes to directory names. */
- size_t i;
+ 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))
{
+ char *new;
+ if (flags & GLOB_MARK)
+ {
size_t len = strlen (pglob->gl_pathv[i]) + 2;
- char *new = realloc (pglob->gl_pathv[i], len);
+ new = realloc (pglob->gl_pathv[i], len);
if (new == NULL)
{
globfree (pglob);
@@ -1129,8 +1135,46 @@ glob (const char *pattern, int flags, int (*errfunc) (const char *, int),
goto out;
}
strcpy (&new[len - 2], "/");
+ }
+ else
+ new = pglob->gl_pathv[i];
+ 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 == 0)
+ {
+ 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[pglob->gl_offs] = new;
+ 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..443f3d467 100644
--- a/tests/test-glob.c
+++ b/tests/test-glob.c
@@ -28,17 +28,180 @@ 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 (k >= g->gl_pathc)
+ break;
+ 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 +249,162 @@ 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, "hello*", "hellod", "hellods", 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_ONLYDIR, "hellod/world*", "hellod/worldd", 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_ONLYDIR, "hello*/world*", "hellod/worldd", "hellods/worldd",
+ 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_ONLYDIR, "hellod/*", "hellod/worldd", 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_ONLYDIR, "*/world*", "hellod/worldd", "hellods/worldd", 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_ONLYDIR | GLOB_BRACE, "{hello*/{,world*},hellof}",
+ "hellod/", "hellods/", "hellod/worldd", "hellods/worldd", 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_ONLYDIR | GLOB_BRACE, "{hello*/{,world*/},hellof}",
+ "hellod/", "hellods/", "hellod/worldd/", "hellods/worldd/", 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_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*/", "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_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);
+
+
+ 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;
}
