[Lldb-commits] [PATCH] D100459: [lldb] [gdb-remote client] Refactor SetCurrentThread*()

Wed Apr 28 02:41:57 PDT 2021

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

In D100459#2721989 <https://reviews.llvm.org/D100459#2721989>, @rovka wrote:

> This looks good to me, but I'll let others comment on whether or not we want to send extra leading zeroes for the thread ID. In my opinion it would be nice to 1) be consistent between all packages that deal with thread ids, and 2) send as little clutter as possible. But these are just theoretical concerns, I'm not sure if it matters in practice.

To be honest, I didn't want to touch it because I don't know if one of the other uses of `PutHex64()` doesn't actually require fixed-width output.

================
Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h:337

+  llvm::Optional<uint64_t> SendSetCurrentThreadPacket(uint64_t tid, char type);
+
----------------
rovka wrote:
> Nitpick: You could call this 'op' instead of 'type', to match the [[ https://sourceware.org/gdb/onlinedocs/gdb/Packets.html#Packets | documentation ]] for the H packet.
Sounds like a good idea. Thanks!

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

https://reviews.llvm.org/D100459