[Lldb-commits] [PATCH] D110025: [lldb] [gdb-remote] Refactor getting remote regs to use local vector

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

Looks good, just be careful about sentinels.



================
Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:571-578
-        if (!value_regs.empty()) {
-          value_regs.push_back(LLDB_INVALID_REGNUM);
-          reg_info.value_regs = value_regs.data();
-        }
-        if (!invalidate_regs.empty()) {
-          invalidate_regs.push_back(LLDB_INVALID_REGNUM);
-          reg_info.invalidate_regs = invalidate_regs.data();
----------------
It looks like you're not doing it here.


================
Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4461-4464
+        if (!reg_info.value_regs.empty())
+          reg_info.value_regs.push_back(LLDB_INVALID_REGNUM);
+        if (!reg_info.invalidate_regs.empty())
+          reg_info.invalidate_regs.push_back(LLDB_INVALID_REGNUM);
----------------
Can we avoid pushing the sentinel here? I'd hope this can be done during the conversion to the "RegisterInfo" format...


================
Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4602
+    uint32_t local_regnum = it.index();
+    RemoteRegisterInfo &remote_reg_info = it.value();
+    // Use remote regnum if available, previous remote regnum + 1 when not.
----------------
i.e., add a `push(sentinel)` here.


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

https://reviews.llvm.org/D110025