[Lldb-commits] [PATCH] D25391: Remove usages of TimeValue from gdb-remote process plugin

[Zachary Turner via lldb-commits]

zturner added inline comments.


================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:192
+    packet_result = ReadPacket(
+        response, std::chrono::duration_cast<std::chrono::microseconds>(
+                      GetPacketTimeout())
----------------
`using namespace std` is generally frowned upon, but I wonder if we could be more lenient about `using namespace std::chrono`?  Would make a lot of those code easier on the eyes.


================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteClientBase.cpp:193-194
+        response, std::chrono::duration_cast<std::chrono::microseconds>(
+                      GetPacketTimeout())
+                      .count(),
+        true);
----------------
Why do you need the `duration_cast` here?  Can't you just pass in the result of `GetPacketTimeout()`, where `GetPacketTimeout()` is updated to return a `std::chrono::duration<int>`?


================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:266-267
+      packet,
+      std::chrono::duration_cast<std::chrono::microseconds>(GetPacketTimeout())
+          .count(),
+      false);
----------------
Same here.  You should just be able to pass the `duration` straight through, and update `ReadPacket` to do the cast.


================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h:86
+    ScopedTimeout(GDBRemoteCommunication &gdb_comm,
+                  std::chrono::seconds timeout);
     ~ScopedTimeout();
----------------
To make this more generic, this could be a `std::chrono::duration<int>`.  If for any reason someone wanted `3.2` seconds, they could then specify it.


https://reviews.llvm.org/D25391