[symbolizer][NFC] Move file argument parsing into separate function

[sepavloff]

Summary

The code that gets binary file name is moved to a separate function.
It makes the code of parseCommand cleaner and allows to reuse the
parsing code.

[ikudrin]

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp

130

The function returns a part of its input, so there is no need to allocate new memory for that, StringRef is enough. It should be the responsibility of the caller to copy the result if necessary.

Also, the function can update Source if it is passed as a reference, which would simplify the code even more.

155–156

It looks like kDelimiters can be removed.

[sepavloff]

Address reviewer's notes

[sepavloff]

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp

130

Indeed, StringRef can be used here.

the function can update Source if it is passed as a reference

Passing by reference can bring new issues. In this case it prevents from using constructs like getSpaceDelimitedWord(Source.ltrim()).

155–156

It is true. Removed.

[ikudrin]

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp

130

Are there any reasons not to do the trimming within this function?

[sepavloff]

Pass Source by reference in getSpaceDelimitedWord

[ikudrin]

LGTM

[jhenderson]

One nit from me, otherwise LGTM too.

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp

137

Nit: https://llvm.org/docs/CodingStandards.html#prefer-preincrement

(I know this was there before, but since you're moving the code, you might as well fix it at the same time)

[sepavloff]

Thanks!

llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp

static void enableDebuginfod(LLVMSymbolizer &Symbolizer,
   IsEnabled = true;
   // Look up symbols using the debuginfod client.
   Symbolizer.setBuildIDFetcher(std::make_unique<DebuginfodFetcher>(
       Args.getAllArgValues(OPT_debug_file_directory_EQ)));
   // The HTTPClient must be initialized for use by the debuginfod client.
   HTTPClient::initialize();
 }
 
+static StringRef getSpaceDelimitedWord(StringRef &Source) {
+  const char kDelimiters[] = " \n\r";
+  const char *Pos = Source.data();
+  StringRef Result;
+  Pos += strspn(Pos, kDelimiters);
+  if (*Pos == '"' || *Pos == '\'') {
+    char Quote = *Pos;
+    Pos++;
+    const char *End = strchr(Pos, Quote);
+    if (!End)
+      return StringRef();
+    Result = StringRef(Pos, End - Pos);
+    Pos = End + 1;
+  } else {
+    int NameLength = strcspn(Pos, kDelimiters);
+    Result = StringRef(Pos, NameLength);
+    Pos += NameLength;
+  }
+  Source = StringRef(Pos, Source.end() - Pos);
+  return Result;
+}
+
 static bool parseCommand(StringRef BinaryName, bool IsAddr2Line,
                          StringRef InputString, Command &Cmd,
                          std::string &ModuleName, object::BuildID &BuildID,
                          uint64_t &ModuleOffset) {
-  const char kDelimiters[] = " \n\r";
   ModuleName = "";
   if (InputString.consume_front("CODE ")) {
     Cmd = Command::Code;
   } else if (InputString.consume_front("DATA ")) {
     Cmd = Command::Data;
   } else if (InputString.consume_front("FRAME ")) {
     Cmd = Command::Frame;
   } else {
     // If no cmd, assume it's CODE.
     Cmd = Command::Code;
   }
 
-  const char *Pos;
   // Skip delimiters and parse input filename (if needed).
   if (BinaryName.empty() && BuildID.empty()) {
     bool HasFilePrefix = false;
     bool HasBuildIDPrefix = false;
     while (true) {
       if (InputString.consume_front("FILE:")) {
         if (HasFilePrefix)
           return false;
         HasFilePrefix = true;
         continue;
       }
       if (InputString.consume_front("BUILDID:")) {
         if (HasBuildIDPrefix)
           return false;
         HasBuildIDPrefix = true;
         continue;
       }
       break;
     }
     if (HasFilePrefix && HasBuildIDPrefix)
       return false;
 
-    Pos = InputString.data();
-    Pos += strspn(Pos, kDelimiters);
-    if (*Pos == '"' || *Pos == '\'') {
-      char Quote = *Pos;
-      Pos++;
-      const char *End = strchr(Pos, Quote);
-      if (!End)
-        return false;
-      ModuleName = std::string(Pos, End - Pos);
-      Pos = End + 1;
-    } else {
-      int NameLength = strcspn(Pos, kDelimiters);
-      ModuleName = std::string(Pos, NameLength);
-      Pos += NameLength;
-    }
+    ModuleName = getSpaceDelimitedWord(InputString);
+    if (ModuleName.empty())
     if (HasBuildIDPrefix) {
       BuildID = parseBuildID(ModuleName);
       if (BuildID.empty())
         return false;
       ModuleName.clear();
     }
   } else {
-    Pos = InputString.data();
     ModuleName = BinaryName.str();
   }
+
   // Skip delimiters and parse module offset.
-  Pos += strspn(Pos, kDelimiters);
-  int OffsetLength = strcspn(Pos, kDelimiters);
-  StringRef Offset(Pos, OffsetLength);
+  InputString = InputString.ltrim();
+  int OffsetLength = InputString.find_first_of(" \n\r");
+  StringRef Offset = InputString.substr(0, OffsetLength);
   // GNU addr2line assumes the offset is hexadecimal and allows a redundant
   // "0x" or "0X" prefix; do the same for compatibility.
   if (IsAddr2Line)
     Offset.consume_front("0x") || Offset.consume_front("0X");
   return !Offset.getAsInteger(IsAddr2Line ? 16 : 0, ModuleOffset);
 }
 
 template <typename T>