[PATCH] [lsan] Implement __lsan_suppress_object().

Kostya Serebryany kcc at google.com
Wed Jun 5 07:10:36 PDT 2013



================
Comment at: lib/lsan/lsan_common.h:156
@@ -155,1 +155,3 @@
 
+enum IgnoreObjectResult {
+  kIgnoreObjectSuccess,
----------------
I am still not getting this enum, I think that bool is much more appropriate given that 
the interface function does not return anything at all. 

================
Comment at: lib/lsan/lsan_common.cc:408
@@ +407,3 @@
+  __lsan::IgnoreObjectLocked(p);
+  // FIXME: in verbose mode, report an error depending on return value
+}
----------------
report the error inside IgnoreObjectLocked and return bool (or even nothing)
Generally, adding a FIXME is a good idea if there is something you can't easily fix now. 
Here is not such case. 

================
Comment at: lib/lsan/lsan_common.h:158
@@ +157,3 @@
+  kIgnoreObjectSuccess,
+  kObjectAlreadyIgnored,
+  kObjectInvalid
----------------
Can you at least have the same prefix in unum values? 
e.g. 
kIgnoreObjectOk, 
kIgnoreObjectAlreadyIgnored, 
kIgnoreObjectInvalid, 

================
Comment at: lib/lsan/lsan_common.cc:408
@@ +407,3 @@
+  __lsan::IgnoreObjectLocked(p);
+  // FIXME: in verbose mode, report an error depending on return value
+}
----------------
Kostya Serebryany wrote:
> report the error inside IgnoreObjectLocked and return bool (or even nothing)
> Generally, adding a FIXME is a good idea if there is something you can't easily fix now. 
> Here is not such case. 
This comment remains. Either fix the FIXME or drop the enum


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



More information about the llvm-commits mailing list