[PATCH] D66613: [support][llvm-objcopy] Add support for shell wildcards

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

>> Also a question, for others as well: is this a feature we actually want to carry forward? Are there any users of this feature/is it useful? This isn't something I've come across as actually being used, though I'm happy to review it if is.

I have the same question.



================
Comment at: llvm/lib/Support/Regex.cpp:214
+
+  int BracketDepth = 0;
+  for (unsigned I = 0, E = Wildcard.size(); I < E; ++I) {
----------------
jhenderson wrote:
> Why is BracketDepth signed? It can't ever be negative.
`BracketDepth` should just be a boolean. There is no nested depth. With the current implementation, the pattern `[[]*` does not match the string `[a`, while in fnmatch(3) (used by GNU objcopy), it matches.

Windows does not have fnmatch(3). If there were a similar function, Unicode might be its weakness...

To correctly parse brackets used in glob, I think the bracket parsing code should detect `[:`, `[.` or `[=` (
collation-related bracket symbols), and skip all characters until the ending symbol is found.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66613/new/

https://reviews.llvm.org/D66613