[Lldb-commits] [PATCH] D62715: [NativeProcessLinux] Reuse memory read by process_vm_readv before calling ptrace

[António Afonso via Phabricator via lldb-commits]

aadsm marked 2 inline comments as done.
aadsm added inline comments.


================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1457
 
-    bytes_read = process_vm_readv(pid, &local_iov, 1, &remote_iov, 1, 0);
-    const bool success = bytes_read == size;
+    auto vm_bytes_read =
+        process_vm_readv(pid, &local_iov, 1, &remote_iov, 1, 0);
----------------
labath wrote:
> llvm's policy is to *not* "almost always use auto".  And it's nice to know whether this returns size_t, ssize_t, or something else without looking up the documentation.
Makes sense. I'll make sure to take a more careful read at https://llvm.org/docs/CodingStandards.html.


================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1467
+      if (!success) {
+        addr = reinterpret_cast<addr_t>(reinterpret_cast<char *>(addr) +
+                                        vm_bytes_read);
----------------
labath wrote:
> `addr += vm_bytes_read` ?
> Since this is talking about addresses in the other process, I can't imagine that casting to a `char *` is going to make this more "correct" in any way.
For some reason I thought that would increase addr by vm_bytes_read*4 or 8 but it's really just a uint64_t not a pointer. I'll also update ReadCStringFromMemory function on my other diff then, I believe I make the same logic there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62715