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

[Alexander Potapenko]

================
Comment at: lib/sanitizer_common/tests/sanitizer_linux_test.cc:52
@@ +51,3 @@
+    if (tid < 0)
+      break;
+    listed_tids.insert(tid);
----------------
I think it's better to keep 'break' on the previous line.

================
Comment at: lib/sanitizer_common/tests/sanitizer_linux_test.cc:49
@@ +48,3 @@
+  std::set<pid_t> listed_tids;
+  while (true) {
+    pid_t tid = thread_lister.GetNextTID();
----------------
How about while(tid = thread_lister.GetNextTID() >= 0) ?

================
Comment at: lib/sanitizer_common/tests/sanitizer_linux_test.cc:63
@@ +62,3 @@
+  *thread_tid = -1;
+  pthread_create_result = pthread_create(thread_id, NULL,
+                                         TIDReporterThread,
----------------
ASSERT_EQ(0, pthread_create(...))

BTW you may find it convenient to write the expected values (0 here) before the actual ones in ASSERT macros.

================
Comment at: lib/sanitizer_common/tests/sanitizer_linux_test.cc:84
@@ -56,3 +83,3 @@
   const uptr kThreadCount = 20; // does not include the main thread
   pthread_t thread_ids[kThreadCount];
   pid_t  thread_tids[kThreadCount];
----------------
Please add a comment that thread_ids and thread_tids are protected by a global mutex (IIUC)

================
Comment at: lib/sanitizer_common/tests/sanitizer_linux_test.cc:106
@@ -100,1 +105,3 @@
 
+  // Check that threads created during ThreadLister object's lifetime become
+  // visible after a Reset() call.
----------------
As discussed offline, there are two or three separate test cases here. Consider using a text fixture (TEST_F)


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