[PATCH] D59597: [scudo][standalone] Add flags & related parsers

[Matt Morehouse via Phabricator via llvm-commits]

morehouse added inline comments.


================
Comment at: lib/scudo/standalone/flags_parser.cc:67
+  const char *Value;
+  if (Buffer[Pos] == '\'' || Buffer[Pos] == '"') {
+    const char Quote = Buffer[Pos++];
----------------
Let's add tests that exercise this path, or just not support quotes.


================
Comment at: lib/scudo/standalone/flags_parser.cc:79
+    if (Buffer[Pos] != 0 && !isSpace(Buffer[Pos]))
+      reportError("expected separator or eol");
+    Value = Buffer + ValueStart;
----------------
Unreachable condition?


================
Comment at: lib/scudo/standalone/flags_parser.cc:127
+    const uptr Len = strlen(Flags[I].Name);
+    if (strncmp(Name, Flags[I].Name, Len) == 0 && Name[Len] == '=') {
+      bool Ok = false;
----------------
Might be cleaner to test the inverse with a continue.


================
Comment at: lib/scudo/standalone/flags_parser.h:21
+enum class FlagType : u8 {
+  FT_bool,
+  FT_int,
----------------
The `FT_` prefix seems redundant since we'll always have to prefix with `FlagType::` anyway.

I suppose it does make things simpler in `registerFlags` though...


================
Comment at: lib/scudo/standalone/tests/flags_test.cc:77
+  int Flag2;
+  ;
+  Parser.registerFlag(Name1, FlagDesc, scudo::FlagType::FT_bool, &Flag1);
----------------
Wild semicolon


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D59597