[PATCH] [compiler-rt] Symbolizer refactoring: Link symbolizer	tools into a fallback chain
    Alexey Samsonov 
    vonosmas at gmail.com
       
    Tue Mar  3 16:30:04 PST 2015
    
    
  
================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_internal.h:31
@@ -30,1 +30,3 @@
  public:
+  SymbolizerTool *next;
+
----------------
You need a SymbolizerTool constructor that would initialize next to `nullptr`. Also, add a comment here why you need it.
================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:353
@@ -352,3 +352,3 @@
  public:
-  explicit POSIXSymbolizer(SymbolizerTool *symbolizer_tool)
-      : Symbolizer(), symbolizer_tool_(symbolizer_tool) {}
+  explicit POSIXSymbolizer() : Symbolizer() {}
+
----------------
Consider just taking IntrusiveList<SymbolizerTool> as an input parameter. Otherwise you:
  * need to call tools_.clear() in constructor anyway
  * introduce an implicit assertion that AppendSymbolizerTool should be called before using any other methods.
================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:372
@@ +371,3 @@
+      if (tool->SymbolizePC(addr, res)) {
+        break;
+      }
----------------
Why not just return here?
================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:392
@@ +391,3 @@
+      if (tool->SymbolizeData(addr, info)) {
+        break;
+      }
----------------
ditto
http://reviews.llvm.org/D8049
EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
    
    
More information about the llvm-commits
mailing list