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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to