[Lldb-commits] [PATCH] D26971: Introduce chrono to more gdb-remote functions

[Pavel Labath via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, zturner, jingham.
labath added a subscriber: lldb-commits.

This replaces the usage of raw integers with duration classes in the gdb-remote
packet management functions. The values are still converted back to integers once
they go into the generic Communication class -- that I am leaving to a separate
change.

The changes are mostly straight-forward (*). The thing I'd most like feedback on
is the representation of timeouts.

Currently, we use UINT32_MAX to denote infinite timeout. This is not well suited
for duration classes, as they tend to do arithmetic on the values, and the
identity of the MAX value can easily get lost (e.g.
microseconds(seconds(UINT32_MAX)).count() != UINT32_MAX). We cannot use zero to
represent infinity (as Listener classes do) because we already use it to do
non-blocking polling reads. For this reason, I believe it is better to have an
explicit value for infinity.

The way I achieved that is via llvm::Optional, and I think it reads quite
natural. Passing llvm::None as "timeout" means "no timeout", while passing zero
means "poll". The only tricky part is this breaks implicit conversions (seconds
are implicitly convertible to microseconds, but Optional<seconds> cannot be
easily converted into Optional<microseconds>). To make this less annoying I have
switched a couple of places from using seconds to microseconds to make the code
flow better.

I think other places could also benefit from the same pattern (e.g., if Listener
functions accepted an llvm::Optional timeout, then we wouldn't need to have
separate Peek*** and Get*** versions of the functions).

(*) The other tricky part was GDBRemoteCommunication::PopPacketFromQueue, which
was needlessly complicated. I've simplified it, but that one is only used in
non-stop mode, and so is untested.


https://reviews.llvm.org/D26971

Files:
  source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServer.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  tools/lldb-server/lldb-platform.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D26971.78865.patch
Type: text/x-patch
Size: 17190 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20161122/e2e08f69/attachment.bin>