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

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.





================
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);
----------------
zturner wrote:
> Is it worth checking the return value of `consumeInteger`?  Is it possible some fields might not be set / do you care about that?
The data comes straight from the OS kernel. I am trusting it to supply the information correctly.


================
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;
----------------
zturner wrote:
> I'd use an early return here, but not a big deal.
I just moved that function around in this patch., I don't want to touch it now.


================
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;
----------------
zturner wrote:
> You can make this a bit shorter with `((llvm::Twine("/proc") + pid) + "/exe").toVector(ProcExe);`
ok


https://reviews.llvm.org/D30942