[PATCH] Add generic ThreadRegistry class for sanitizer runtimes.

Kostya Serebryany kcc at google.com
Tue Mar 12 05:54:38 PDT 2013



================
Comment at: lib/sanitizer_common/sanitizer_thread_registry.h:46
@@ +45,3 @@
+  uptr user_id;   // Some opaque user thread id (e.g. pthread_t).
+  char *name;     // As annotated by user.
+
----------------
fixed size buffer, please

================
Comment at: lib/sanitizer_common/sanitizer_thread_registry.h:57
@@ +56,3 @@
+    if (name) {
+      InternalFree(name);
+    }
----------------
no InternalFree as long as it calls libc, please 

================
Comment at: lib/sanitizer_common/sanitizer_thread_registry.h:163
@@ +162,3 @@
+      if (dead_threads_.size() == 0) {
+        Report("Thread limit (%u threads) exceeded. Dying.\n", MaxThreads);
+        Die();
----------------
if you Report something, please also print the tool name (SanitizerToolName)

================
Comment at: lib/sanitizer_common/sanitizer_thread_registry.h:94
@@ +93,3 @@
+    // Parent tid makes no sense for the main thread.
+    if (tid != 0) {
+      parent_tid = _parent_tid;
----------------
we now tend to omit {} in such cases. 

================
Comment at: lib/sanitizer_common/sanitizer_thread_registry.h:211
@@ +210,3 @@
+    for (u32 tid = 0; tid < thread_id_; tid++) {
+    //for (u32 tid = 0; tid < MaxThreads; tid++) {
+      TCTX *tctx = threads_[tid];
----------------
remove this comment? 

================
Comment at: lib/sanitizer_common/sanitizer_thread_registry.h:224
@@ +223,3 @@
+    for (u32 tid = 0; tid < thread_id_; tid++) {
+    //for (u32 tid = 0; tid < MaxThreads; tid++) {
+      TCTX *tctx = threads_[tid];
----------------
remove this comment?

================
Comment at: lib/sanitizer_common/sanitizer_thread_registry.h:120
@@ +119,3 @@
+
+  u32 thread_id_;
+  uptr unique_thread_id_;
----------------
looks like a bad name for this var. 
first_unused_thread_id? 


================
Comment at: lib/sanitizer_common/sanitizer_thread_registry.h:121
@@ +120,3 @@
+  u32 thread_id_;
+  uptr unique_thread_id_;
+  TCTX *threads_[MaxThreads];
----------------
ditto


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



More information about the llvm-commits mailing list