[Lldb-commits] [PATCH] D30942: Remove some ProcFileReader occurences

[Zachary Turner via Phabricator via lldb-commits]

zturner added a comment.

Code looks much nicer than before!



================
Comment at: source/Host/linux/Host.cpp:59-60
 
-  static const char tracerpid_token[] = "TracerPid:";
-  char *buf_tracerpid = strstr((char *)buf_sp->GetBytes(), tracerpid_token);
-  if (buf_tracerpid) {
-    // Tracer PID. 0 if we're not being debugged.
-    buf_tracerpid += sizeof(tracerpid_token);
-    tracerpid = strtol(buf_tracerpid, &buf_tracerpid, 10);
+  for (llvm::StringRef Line, Rest = BufferOrError.get()->getBuffer();
+       !Rest.empty(); std::tie(Line, Rest) = Rest.split('\n')) {
+    if (Line.consume_front("Gid:")) {
----------------
Random thought.  Someone should make a `LinesIterator`.  (Not you, just a random thought like I said)


================
Comment at: source/Host/linux/Host.cpp:63-70
+      Line = Line.ltrim();
+      uint32_t RGid, EGid;
+      Line.consumeInteger(10, RGid);
+      Line = Line.ltrim();
+      Line.consumeInteger(10, EGid);
+
+      ProcessInfo.SetGroupID(RGid);
----------------
Is it worth checking the return value of `consumeInteger`?  Is it possible some fields might not be set / do you care about that?


================
Comment at: source/Host/linux/Host.cpp:145
+  assert(num_specs <= 1 && "Linux plugin supports only a single architecture");
+  if (num_specs == 1) {
+    ModuleSpec module_spec;
----------------
I'd use an early return here, but not a big deal.


================
Comment at: source/Host/linux/Host.cpp:166
+  llvm::SmallString<64> ProcExe;
+  (llvm::Twine("/proc/") + llvm::Twine(pid) + "/exe").toVector(ProcExe);
+  llvm::SmallString<0> ExePath;
----------------
You can make this a bit shorter with `((llvm::Twine("/proc") + pid) + "/exe").toVector(ProcExe);`


https://reviews.llvm.org/D30942