[PATCH] fixed ThreadLister test for the case of pre-existing threads and added a test for ThreadLister::Reset()

Alexander Potapenko glider at google.com
Thu Mar 14 04:34:50 PDT 2013


  Looks mostly good.


================
Comment at: lib/sanitizer_common/tests/sanitizer_linux_test.cc:44
@@ +43,3 @@
+    pthread_cond_init(&tid_reported_cond, NULL);
+
+    terminate_thread = false;
----------------
Please remove the spare newline.

================
Comment at: lib/sanitizer_common/tests/sanitizer_linux_test.cc:30
@@ +29,3 @@
+struct TidReporterArgument {
+  pid_t reported_tid;
+  // For signaling to spawned threads that they should terminate.
----------------
Please move the struct members after the methods.

================
Comment at: lib/sanitizer_common/tests/sanitizer_linux_test.cc:82
@@ +81,3 @@
+
+  std::vector<pthread_t> thread_ids_;
+  std::vector<pid_t> thread_tids_;
----------------
FYI these two variables essentially mean "thread identifiers" and "thread thread identifiers".

================
Comment at: lib/sanitizer_common/tests/sanitizer_linux_test.cc:61
@@ +60,3 @@
+    pid_t thread_tid;
+
+    for (uptr i = 0; i < kThreadCount; i++) {
----------------
This one is spare, too, I think.


http://llvm-reviews.chandlerc.com/D532



More information about the llvm-commits mailing list