Add Path::glob() function

doc/corrade-changelog.dox

@@ -137,6 +137,8 @@ namespace Corrade {
 -   New @ref Corrade/Utility/Math.h header implementing @ref Utility::min() and
     @ref Utility::max() because having to @cpp #include <algorithm> @ce to get
     @ref std::min() and @ref std::max() is unacceptable.
+-   New @ref Utility::Path::glob() function for listing directory contents
+    matching a wildcard pattern
 -   Added @ref Utility::String::lowercaseInPlace() and @relativeref{Utility::String,uppercaseInPlace()}
     together with @ref Utility::String::lowercase() and
     @relativeref{Utility::String,uppercase()} overloads taking a

src/Corrade/PluginManager/AbstractManager.cpp

@@ -428,16 +428,12 @@ void AbstractManager::setPluginDirectory(const Containers::StringView directory)
         when the directory doesn't exist, as a lot of existing code and tests
         relies on it. Figure out a better solution. */
     if(Utility::Path::exists(_state->pluginDirectory)) {
-        Containers::Optional<Containers::Array<Containers::String>> d = Utility::Path::list(
-            _state->pluginDirectory,
-            Utility::Path::ListFlag::SkipDirectories|
-            Utility::Path::ListFlag::SkipDotAndDotDot|
-            Utility::Path::ListFlag::SortAscending);
+        Containers::Optional<Containers::Array<Containers::String>> d = Utility::Path::glob(
+            Utility::Path::join(_state->pluginDirectory, "*"_s + _state->pluginSuffix),
+            Utility::Path::GlobFlag::SkipDirectories|
+            Utility::Path::GlobFlag::SortAscending);
+        if(d) !Utility::Debug{} << d;
         if(d) for(const Containers::StringView filename: *d) {
-            /* File doesn't have module suffix, continue to next */
-            if(!filename.hasSuffix(_state->pluginSuffix))
-                continue;
-
             /* Dig plugin name from filename */
             const Containers::StringView name = filename.exceptSuffix(_state->pluginSuffix);
 

src/Corrade/PluginManager/Test/ManagerTest.cpp

@@ -273,7 +273,7 @@ void ManagerTest::pluginDirectoryNotReadable() {
         PluginManager::Manager<SomePlugin> manager{directory};
     }
     CORRADE_COMPARE_AS(out.str(),
-        Utility::formatString("Utility::Path::list(): can't list {}: error ", directory),
+        Utility::formatString("Utility::Path::glob(): can't glob {}/*{}: error ", directory, AbstractPlugin::pluginSuffix()),
         TestSuite::Compare::StringHasPrefix);
 }
 

src/Corrade/Utility/Path.cpp

@@ -64,6 +64,7 @@
 #include <cstring>
 #include <sys/stat.h>
 #include <dirent.h>
+#include <glob.h>
 #include <unistd.h>
 #ifdef CORRADE_TARGET_APPLE
 #include <mach-o/dyld.h>
@@ -750,16 +751,160 @@ Containers::Optional<Containers::Array<Containers::String>> list(const Container
 
     closedir(directory);
 
+    if(flags & (ListFlag::SortAscending|ListFlag::SortDescending))
+        std::sort(list.begin(), list.end());
+    /* We don't have rbegin() / rend() on Array (would require a custom
+       iterator class or a StridedArrayView), so just reverse the result */
+    if(flags >= ListFlag::SortDescending && !(flags >= ListFlag::SortAscending))
+        std::reverse(list.begin(), list.end());
+
+    /* GCC 4.8 and Clang 3.8 need extra help here */
+    return Containers::optional(std::move(list));
+
+    /* Windows (not Store/Phone) is implemented via glob(), a subset of the
+       flag bits supported by both is expected to match */
+    #elif defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT)
+    return glob(join(path, "*"_s), GlobFlag(static_cast<unsigned char>(flags)));
+    #else
+    #error
+    #endif
+
+    /* Other not implemented */
+    #else
+    Error{} << "Utility::Path::list(): not implemented on this platform";
+    static_cast<void>(path);
+    return {};
+    #endif
+}
+
+Containers::Optional<Containers::Array<Containers::String>> glob(const Containers::StringView pattern, GlobFlags flags) {
+    #if defined(CORRADE_TARGET_UNIX) || defined(CORRADE_TARGET_EMSCRIPTEN) || (defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT))
+    /* POSIX-compliant Unix, Emscripten */
+    #if defined(CORRADE_TARGET_UNIX) || defined(CORRADE_TARGET_EMSCRIPTEN)
+    /* Don't allow escaping * and ? with \ for consistency with Windows and to
+       avoid clashing with directory separators on Windows.
+
+       Also fail when encountering an, which means it'd fail also when stat()
+       on any file would fail, which is a different behavior from Path::list().
+       There's no way to separate the two cases except for attaching the error
+       handler and then ... um ... trying to figure out whether the failure
+       happened during opendir() or during stat()? The error handler carries no
+       state pointer, so that would be excessively shitty.
+
+       Without GLOB_ERR set, it would just try to carry on, ultimately claiming
+       a GLOB_NOMATCH. And a GLOB_NOMATCH is not an error state as it happens
+       when globbing an existing but empty directory. */
+    int globFlags = GLOB_NOESCAPE|GLOB_ERR;
+    /* The output is sorted by default which means we don't have to do that.
+       Disable it if not requested as an optimization. */
+    if(!(flags & (GlobFlag::SortAscending|GlobFlag::SortDescending)))
+        globFlags |= GLOB_NOSORT;
+    glob_t out;
+    /* With musl (and with Emscripten that uses musl), an error from opening a
+       directory (such as when globbing `nonexistent/``*`) that happens at:
+        https://github.com/bminor/musl/blob/6d8a515796270eb6cec8a278cb353a078a10f09a/src/regex/glob.c#L127-L130
+       gets ignored by the code path that haldes the case of no matches, and so
+       we get GLOB_NOMATCH instead:
+        https://github.com/bminor/musl/blob/6d8a515796270eb6cec8a278cb353a078a10f09a/src/regex/glob.c#L261-L270
+       Fortunately the opendir() failure sets errno which we can subsequently
+       check... unless it's a version before
+        https://github.com/emscripten-core/emscripten/commit/e05e72d9c49fe15578e73934ce525a894d1b712a
+       which apparently neither sets the errno nor calls the error handler, so
+       we have no way to check anything. Apart from that, this is still not
+       100% bulletproof as is possible that something else would set errno even
+       in the successful scenario. */
+    errno = 0;
+    const int result = glob(Containers::String::nullTerminatedView(pattern).data(), globFlags, nullptr, &out);
+    /* Having no results is fine, but only if we're not under musl that has the
+       bug explained above, so check errno as well */
+    if(result == GLOB_NOMATCH && !errno)
+        return Containers::Array<Containers::String>{};
+    if(result != 0) {
+        Error err;
+        err << "Utility::Path::glob(): can't glob" << pattern << Debug::nospace << ":";
+        Utility::Implementation::printErrnoErrorString(err, errno);
+        return {};
+    }
+
+    Containers::Array<Containers::String> list;
+
+    for(std::size_t i = 0; i != out.gl_pathc; ++i) {
+        /* If the pattern was a full path, the output returns a full path as
+           well. For consistency with list() and with the Windows API we return
+           just a filename portion. */
+        const Containers::StringView file = split(out.gl_pathv[i]).second();
+        if((flags >= GlobFlag::SkipDotAndDotDot) && (file == "."_s || file == ".."_s))
+            continue;
+
+        /* Compared to readdir() we don't implicitly get any entry type here,
+           so we have bear an extra overhead of stat() for every returned path
+           -- thus do it only if we're told to skip anything. If it fails for
+           the particular entry, treat it as "neither a file nor a directory"
+           and leave it in the list -- we're told to skip files/directories,
+           not "include only files/directories". */
+        if(flags & (GlobFlag::SkipDirectories|GlobFlag::SkipFiles|GlobFlag::SkipSpecial)) {
+            /* stat() follows the symlink, lstat() doesn't. This is equivalent
+               to what's done in Path::list(), except that here it's always and
+               not just for symlinks. */
+            struct stat st;
+            /* gl_pathv[i] is absolute if pattern was absolute, so no need to
+               prepend it again */
+            if(stat(out.gl_pathv[i], &st) == 0) {
+                if(flags >= GlobFlag::SkipDirectories && S_ISDIR(st.st_mode))
+                    continue;
+                if(flags >= GlobFlag::SkipFiles && S_ISREG(st.st_mode))
+                    continue;
+                if(flags >= GlobFlag::SkipSpecial && !S_ISDIR(st.st_mode) && !S_ISREG(st.st_mode))
+                    continue;
+            }
+        }
+
+        arrayAppend(list, file);
+    }
+
+    globfree(&out);
+
     /* Windows (not Store/Phone) */
     #elif defined(CORRADE_TARGET_WINDOWS) && !defined(CORRADE_TARGET_WINDOWS_RT)
     WIN32_FIND_DATAW data;
-    /** @todo drop the StringView cast once widen(const std::string&) is
-        removed */
-    HANDLE hFile = FindFirstFileW(Unicode::widen(Containers::StringView{join(path, "*"_s)}), &data);
+    /* Originally this used FindFirstFileW(), but that one implicitly includes
+       8.3 filenames in the match. That sounded just like an unnecessary but
+       harmless work at first, but turns out it affects how files with
+       extensions longer than 3 characters are processed. So if globbing for
+       `*.txt` and there's a `*.txtlol` file, it'd match it as well, because
+       its 8.3 form is something like FILE~1.TXT. Haha. */
+    /** @todo there's also FIND_FIRST_EX_CASE_SENSITIVE, expose? */
+    HANDLE hFile = FindFirstFileExW(Unicode::widen(pattern), FindExInfoBasic, &data, FindExSearchNameMatch, nullptr, 0);
     if(hFile == INVALID_HANDLE_VALUE) {
+        /* If we're matching `path/``*` and it returns ERROR_FILE_NOT_FOUND, it
+           means the path doesn't exist -- otherwise it would return at least a
+           `..`. This is a valid reason to error.
+
+           If we're however matching e.g. `path/``*.txt` and it returns
+           ERROR_FILE_NOT_FOUND, we should not error if `path` is an existing directory that just doesn't contain any text files. So then we
+           subsequently check if the path is a directory, and if it is, we
+           return an empty list, consistently with the Unix implementation
+           above. */
+        const unsigned int error = GetLastError();
+        if(error == ERROR_FILE_NOT_FOUND) {
+            const Containers::Pair<Containers::StringView, Containers::StringView> pathFilename = split(pattern);
+            /* Yes, this performs a second UTF-8 -> UTF-16 conversion
+               internally, because we need a smaller null-terminated string. An
+               alternative would be to cache the Unicode::widen(pattern) from
+               above, then somehow add a 16-bit zero to where the last 16-bit /
+               is, and then call the Windows API directly, but who wants to
+               write extra tests for all that? I don't. */
+            if(pathFilename.second() != "*"_s && isDirectory(pathFilename.first()))
+                return Containers::Array<Containers::String>{};
+        }
+
         Error err;
-        err << "Utility::Path::list(): can't list" << path << Debug::nospace << ":";
-        Utility::Implementation::printWindowsErrorString(err, GetLastError());
+        err << "Utility::Path::glob(): can't glob" << pattern << Debug::nospace << ":";
+        /* Using the cached error from above and not GetLastError() in case
+           the isDirectory() call would produce another error internally. The
+           isDirectory() call also shouldn't be printing anything on its
+           own. */
+        Utility::Implementation::printWindowsErrorString(err, error);
         return {};
     }
     Containers::ScopeGuard closeHandle{hFile,
@@ -773,44 +918,59 @@ Containers::Optional<Containers::Array<Containers::String>> list(const Container
 
     Containers::Array<Containers::String> list;
 
-    /* Explicitly add `.` for compatibility with other systems */
-    if(!(flags & (ListFlag::SkipDotAndDotDot|ListFlag::SkipDirectories)))
-        arrayAppend(list, "."_s);
+    /* Explicitly add `.` for compatibility with other systems if the glob
+       pattern would include it. Windows doesn't have any special treatment for
+       dotfiles, so `*` should match it as well, the same as it matches
+       `..`. */
+    if(!(flags & (GlobFlag::SkipDotAndDotDot|GlobFlag::SkipDirectories))) {
+        const Containers::StringView wildcard = split(pattern).second();
+        /** @todo not robust enough, ?, ** or .***** matches them as well */
+        if(wildcard == "*"_s || wildcard == ".*"_s)
+            arrayAppend(list, "."_s);
+    }
 
     while(FindNextFileW(hFile, &data) != 0 || GetLastError() != ERROR_NO_MORE_FILES) {
-        if((flags >= ListFlag::SkipDirectories) && (data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY))
+        if((flags >= GlobFlag::SkipDirectories) && (data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY))
             continue;
-        if((flags >= ListFlag::SkipFiles) && !(data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY))
+        if((flags >= GlobFlag::SkipFiles) && !(data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY))
             continue;
         /** @todo symlink support */
         /** @todo are there any special files in WINAPI? */
 
-        /* Not testing for dot, as it is not listed on Windows. Also it doesn't
-           cause any unnecessary temporary allocation if SkipDotAndDotDot is
-           used because `..` fits easily into SSO. */
+        /* Not testing for `.`, as it is not listed on Windows -- we explicitly
+           added it above if it made sense. We could also check for the UTF-16
+           variant before creating a String instance, but `..` fits easily into
+           SSO so there's no unnecessary allocation being done. Plus `..`
+           appears just once for the whole call, so that's a constant overhead
+           factor. */
         Containers::String file = Unicode::narrow(data.cFileName);
-        if((flags >= ListFlag::SkipDotAndDotDot) && file == ".."_s)
+        if((flags >= GlobFlag::SkipDotAndDotDot) && file == ".."_s)
             continue;
 
         arrayAppend(list, std::move(file));
     }
+
+    /* Sorting done just here, Unix glob() can do it on its own so that's what
+       we use there instead. Reversal of the sorted list is done below for
+       both, however. */
+    if(flags & (GlobFlag::SortAscending|GlobFlag::SortDescending))
+        std::sort(list.begin(), list.end());
     #else
     #error
     #endif
 
-    if(flags & (ListFlag::SortAscending|ListFlag::SortDescending))
-        std::sort(list.begin(), list.end());
-    /* We don't have rbegin() / rend() on Array (would require a custom
-       iterator class or a StridedArrayView), so just reverse the result */
-    if(flags >= ListFlag::SortDescending && !(flags >= ListFlag::SortAscending))
+    /* On Unix the sorting was done by glob() already, on Windows via
+       std::sort(), so it's just the descending order left, which is done by
+       reversing the sorted list. */
+    if(flags >= GlobFlag::SortDescending && !(flags >= GlobFlag::SortAscending))
         std::reverse(list.begin(), list.end());
 
     /* GCC 4.8 and Clang 3.8 need extra help here */
     return Containers::optional(std::move(list));
 
     /* Other not implemented */
     #else
-    Error{} << "Utility::Path::list(): not implemented on this platform";
+    Error{} << "Utility::Path::glob(): not implemented on this platform";
     static_cast<void>(path);
     return {};
     #endif