[Lldb-commits] [PATCH] D77043: Use remote regnums in expedited list, value regs and invalidate regs

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp:404
                reg = reg_info->invalidate_regs[++idx]) {
+            uint32_t lldb_regnum = ConvertRegisterKindToRegisterNumber(
+                eRegisterKindProcessPlugin, reg);
----------------
omjavaid wrote:
> Using ConvertRegisterKindToRegisterNumber because SetRegisterIsValid requires register number as input unlike other instances where we needed to get register info.
That's fine -- what bothers me is the conditional setting of the `reg` variable below.

What I am pushing for is for consistency in the meanings of `invalidate_regs` numbers. If the invalidate_regs always stores the "process plugin" register kind, then calling we should never be calling`SetRegisterIsValid(reg)` if we fail to convert the plugin number to a "register number". 
I would expect something like `SetRegisterIsValid(ConvertRegisterKindToRegisterNumber(eRegisterKindProcessPlugin, reg), false)` (with some error checking if it can really happen than `ConvertRegisterKindToRegisterNumber` can fail here, but I hope that's not needed);


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

https://reviews.llvm.org/D77043