[Lldb-commits] [PATCH] D20436: Clean up vestigial remnants of locking primitives

Saleem Abdulrasool via lldb-commits lldb-commits at lists.llvm.org
Mon Jul 18 18:55:39 PDT 2016


compnerd added a comment.

Ugh, yeah, I had forgotten about this.  Ill try to get to this tonight/tomorrow.


================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp:354-355
@@ -358,5 +353,4 @@
 {
-    // Calculate absolute timeout value
-    TimeValue timeout = TimeValue::Now();
-    timeout.OffsetWithMicroSeconds(timeout_usec);
+    std::chrono::time_point<std::chrono::system_clock, std::chrono::microseconds> until =
+        std::chrono::system_clock::now() + std::chrono::microseconds(timeout_usec);
 
----------------
zturner wrote:
> Need to use `auto` here, otherwise there's a compiler error.
Yeah, noticed that on Linux; Ill upload a new version of the patch.  I ended up just doing the math in a second step.

================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:844
@@ -845,1 +843,3 @@
+                        if (m_async_packet_predicate.WaitForValueEqualTo(
+                                false, until - std::chrono::system_clock::now(), &timed_out))
                         {
----------------
zturner wrote:
> this needs to use `std::chrono::duration_cast<std::chrono::microseconds>(until - std::chrono::system_clock::now())`.  Maybe raise this into a temporary variable (also make sure to use auto on the result just in case).
Yeah, hoisting that into a local var sounds good.

================
Comment at: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:860
@@ -859,2 +859,3 @@
                         // Make sure we wait until the continue packet has been sent again...
-                        if (m_private_is_running.WaitForValueEqualTo (true, &timeout_time, &timed_out))
+                        if (m_private_is_running.WaitForValueEqualTo(true, until - std::chrono::system_clock::now(),
+                                                                     &timed_out))
----------------
zturner wrote:
> `duration_cast` again.
Yeah.

================
Comment at: source/Target/Process.cpp:5547
@@ +5546,3 @@
+                    log->Printf("Process::RunThreadPlan(): about to wait - now is %llu - endpoint is %llu",
+                                std::chrono::time_point<std::chrono::system_clock, std::chrono::microseconds>(
+                                    std::chrono::system_clock::now())
----------------
zturner wrote:
> Delete the cast here.  Just use `std::chrono::system_clock::now()`.
Yeah.

================
Comment at: source/Target/Process.cpp:5551
@@ +5550,3 @@
+                                    .count(),
+                                std::chrono::time_point<std::chrono::system_clock, std::chrono::microseconds>(timeout)
+                                    .time_since_epoch()
----------------
zturner wrote:
> Change to `timeout.time_since_epoch()`
This doesn't work for me.  `timeout` is a `std::duration`, and `std::time_point` is needed.


Repository:
  rL LLVM

https://reviews.llvm.org/D20436





More information about the lldb-commits mailing list