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



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

Reply via email to