[Lldb-commits] [PATCH] D68622: IOHandler: fall back on File::Read if a FILE* isn't available.

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Core/IOHandler.cpp:317-321
+  size_t end = pos + 1;
+  while (end > 0 &&
+         (line_buffer[end - 1] == '\n' || line_buffer[end - 1] == '\r'))
+    end--;
+  std::string line = line_buffer.substr(0, end);
----------------
`line = StringRef(line_buffer.begin(), pos).rtrim("\n\r")` ?


================
Comment at: lldb/source/Core/IOHandler.cpp:326-341
+// Append newchars to the buffer and split out a line.
+static Optional<std::string> AppendAndSplitLine(std::string &line_buffer,
+                                                StringRef newchars) {
+  size_t pos = newchars.find('\n');
+  if (pos == StringRef::npos) {
+    line_buffer.append(newchars);
+    return None;
----------------
This looks like too much optimization. Why not just append the new stuff and then call `SplitLine` ?


================
Comment at: lldb/source/Core/IOHandler.cpp:346-347
+static Optional<std::string> SplitLineEOF(std::string &line_buffer) {
+  if (line_buffer.empty())
+    return None;
+  if (std::all_of(line_buffer.begin(), line_buffer.end(), isspace))
----------------
Technically, this check is subsumed by the next one.


================
Comment at: lldb/source/Core/IOHandler.cpp:348
+    return None;
+  if (std::all_of(line_buffer.begin(), line_buffer.end(), isspace))
+    return None;
----------------
`llvm::all_of(line_buffer, ::isspace)`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68622