[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