[Lldb-commits] [PATCH] D70417: Accept g packet responses that don't supply all registers

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda marked an inline comment as done.
jasonmolenda added a comment.

Thanks for looking this over Pavel.



================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp:215
+            struct RegisterInfo *reginfo = m_reg_info.GetRegisterInfoAtIndex(i);
+            if (reginfo->byte_offset < buffer_sp->GetByteSize()) {
+              m_reg_valid[i] = true;
----------------
labath wrote:
> Should this be something like `reginfo->byte_offset+reg_info->byte_size < ...` ?
Yeah, that was my first thought too, then I thought, SURELY we'll have the correct # of bytes for complete registers, even if it's less registers than expected.  But that's maybe not a great assumption.  I'll change it.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteRegisterContext.cpp:215-220
+            if (reginfo->byte_offset + reginfo->byte_size 
+                   <= buffer_sp->GetByteSize()) {
+              m_reg_valid[i] = true;
+            } else {
+              m_reg_valid[i] = false;
+            }
----------------
labath wrote:
> maybe just `m_reg_valid[i] = reginfo->byte_offset + reginfo->byte_size <= buffer_sp->GetByteSize()`
I think I like the more verbose form, but I don't feel strongly about it.  It takes up more screen real estate but I think it's easier to understand at a glance - purely a personal opinion.  I'm sure they compile to the same code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70417