Glob code refactoring #2137
Merged
Glob code refactoring #2137
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Depends on #2151 so marking this one as a draft until it's merged. |
dmnks
force-pushed
the
iss1749-refactor
branch
from
0286967 to
e86c6f6
Compare
dmnks
force-pushed
the
iss1749-refactor
branch
from
e86c6f6 to
73e6432
Compare
Instead of constructing a new list from scratch and returning that, just extend the passed list. This makes it easier to use this function incrementally when expanding multiple patterns in a loop, such as during package manifest parsing which we adapt here right away. Preserve the ability to pass NULL as argvPtr and still get a match count via argcPtr, by keeping the local argv around for that case. No functional change.
dmnks
force-pushed
the
iss1749-refactor
branch
from
73e6432 to
6e1279b
Compare
|
Rebased, ready for review now. |
By definition, glob(3) matches pathnames on the file system, so no pattern starting with a URL protocol (e.g. http:// or file://) will ever produce any meaningful results when passed to it, it will just fail with GLOB_NOMATCH. This wasn't always the case, we used to call a custom Glob() function here in the past, which knew how to handle URLs, but that was axed in commit 9cbf034 some 15 years ago. To this day, however, we somewhat continue the legacy by letting URL_IS_PATH (file://) patterns pass through glob(3) if they contain magic chars, with the only possible outcome of failing afterwards. Drop this special case and simply consider any known URL pattern as non-local (int local = 0) and return it immediately. Also remove the no-op URL code while at it.
Rather than listing all the rules, just refer the reader to the man page. Although brace expansion isn't part of the standard rules, we still support it through glob(3) with GLOB_BRACE, so make a note.
Do set *argcPtr even if the actual count is 0, rather than leaving it unchanged and thus possibly undefined in the caller. This is also consistent with how glob(3) works.
As per GNU glob(3), GLOB_ONLYDIR does not guarantee the matches are in fact directories, that's why we check them with lstat(2). That may lead to the match list being empty even after a successful glob() run (rc == 0), so for consistency, return GLOB_NOMATCH in that case, just like we would for a missing file. No functional change since we don't check for the exact return code in the callers, only whether it's positive or not.
No functional change, just shuffle some lines around for better readability.
dmnks
force-pushed
the
iss1749-refactor
branch
from
6e1279b to
f667dd3
Compare
|
Nice set of cleanups! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Clean up some old cruft, update the docs and prepare ground for the following PR.