[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