[Lldb-commits] [PATCH] D156687: [lldb][AArch64] Add kind marker to ReadAll/WriteALLRegisterValues data

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added inline comments.


================
Comment at: lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:573
 
-  ::memcpy(dst, GetGPRBuffer(), GetGPRBufferSize());
-  dst += GetGPRBufferSize();
+  dst = AddSavedRegisters(dst, SavedRegistersKind::GPR, GetGPRBuffer(),
+                          GetGPRBufferSize());
----------------
omjavaid wrote:
> I think we have lost some of code readability of ReadAllRegisterValues function by introducing AddSavedRegisters mechanism. It does remove some duplication of memcpy but it also introduces some differentiation between Read/WritellRegisterValues functions. I am inclined that we should put the memcpy into ReadAllRegisterValues to make it look similar to its Write counterpart. 
I added it to avoid repeating:
```
  Add kind whatever
  ::memcpy(dst, src, size);
  dst = dst + size;
```
Or rather, not repeating it because it was very easy to forget to advance the pointer. The same applies to WriteAllRegisterValues, but there is some more logic in between the steps so I didn't do it there.

If memcpy returned a pointer one beyond the end of the buffer I wouldn't mind doing:
```
Add kind
dst = memcpy(...)
```

But it only returns the original destination pointer.

So I'd rather not inline all the memcopies here but I could try harder to add a convenience function for WriteAllRegisterValues, so they more closely match.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156687