[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add MTE memory tag reading to lldb-server

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

  To get the review going consider further splitting up these patches into separate chunks containing, for example MemoryTagHandler API and related unit tests, GDB remote addons, ptrace work etc. Also add reference to GDB mailing list discussion where design of qMemTag packet has been discussed.

Will do. I erred on the side of larger patches so you could see the pieces connect in the same review.

  Soft suggestion to rename MemoryTagHandler, IMO it sounds more like a routine rather than a set of helpers used for manipulating memory tags. Also lldb/include/lldb/Target doesnt seem like a good place for helper functions as MemoryTagHandler is not used as Memory Tag container class. May be consider putting it under source/Plugins/Process/Utility

How about "MemoryTagInterface"? It's very similar to a Java "interface" class.

I think my reasoning for putting the header in lldb/Target was because it's used in the client and the server, and one of them tends not to include from the latter location. I'll double check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95601