[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

138

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.

164

It looks like kDelimiters can be removed.

[sepavloff]

Address reviewer's notes

[sepavloff]

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

138

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()).

164

It is true. Removed.

[ikudrin]

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

138

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

145

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 std::pair<std::string, StringRef>
+getSpaceDelimitedWord(StringRef Source) {
+  const char kDelimiters[] = " \n\r";
+  const char *Pos = Source.data();
+  std::string Result;
+  Pos += strspn(Pos, kDelimiters);
+  if (*Pos == '"' || *Pos == '\'') {
+    char Quote = *Pos;
+    Pos++;
+    const char *End = strchr(Pos, Quote);
+    if (!End)
+      return std::make_pair(Result, Source);
+    Result = std::string(Pos, End - Pos);
+    Pos = End + 1;
+  } else {
+    int NameLength = strcspn(Pos, kDelimiters);
+    Result = std::string(Pos, NameLength);
+    Pos += NameLength;
+  }
+  return std::make_pair(Result, StringRef(Pos, Source.end() - Pos));
+}
+
 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;
-    }
+    std::tie(ModuleName, InputString) = 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(kDelimiters);
+  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>