[PATCH] Generalize stackdepot.

Dmitry Vyukov dvyukov at google.com
Sun May 18 23:58:05 PDT 2014


================
Comment at: sanitizer_common/sanitizer_chainedorigindepot.h:22
@@ +21,3 @@
+
+StackDepotStats *ChainedOriginDepotGetStats();
+u32 ChainedOriginDepotPut(u32 here_id, u32 prev_id);
----------------
Why doesn't this live in msan dir now? This looks msan-specific.

================
Comment at: sanitizer_common/sanitizer_region_allocator.h:1
@@ +1,2 @@
+//===-- sanitizer_region_allocator.h ----------------------------*- C++ -*-===//
+//
----------------
please use "svn copy" for this file to preserve history.

================
Comment at: sanitizer_common/sanitizer_region_allocator.h:23
@@ +22,3 @@
+
+class RegionAllocator {
+ public:
----------------
I would call it "PersistentAllocator" rather than RegionAllocator. Region allocators usually assume existence of plurality of regions (e.g. one per each incoming server request). Region allocators also usually provide region free operations.


================
Comment at: sanitizer_common/sanitizer_stackdepot.h:18
@@ -17,2 +17,3 @@
 #include "sanitizer_internal_defs.h"
+#include "sanitizer_stackdepotbase.h"
 
----------------
does this need to be included in the header file?
it would be nice if template classes are included only in source files.

================
Comment at: sanitizer_common/sanitizer_stackdepotbase.h:1
@@ +1,2 @@
+//===-- sanitizer_stackdepotbase.h ------------------------------*- C++ -*-===//
+//
----------------
svn copy as well

http://reviews.llvm.org/D3790






More information about the llvm-commits mailing list