<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Mar 15, 2013 at 9:35 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
================<br>
Comment at: lib/sanitizer_common/tests/sanitizer_allocator_test.cc:571<br>
@@ +570,3 @@<br>
<div class="im">+ public:<br>
+  explicit IterationTestCallback(std::set<void *> *chunks)<br>
+    : chunks_(chunks) {}<br>
</div>----------------<br>
(not important, but just a thought)<br>
<br>
The LLVM project codebase doesn't really have the convention to pass mutable data by pointer (we're usually more than happy to pass by non-const ref). Not sure if the sanitizer code has adopted this convention consistently/separately from the rest of LLVM.<br>

<br>
================<br>
Comment at: lib/sanitizer_common/tests/sanitizer_allocator_test.cc:610<br>
@@ +609,3 @@<br>
+  a->ForceLock();<br>
<div class="im">+  a->template ForEachChunk<IterationTestCallback>(callback);<br>
+  a->ForceUnlock();<br>
</div>----------------<br>
Any reason you can't just call this with "a->ForEachChunk(callback)", relying on template argument deduction?<br></blockquote><div><br></div><div style>Fixed in r177245</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<br>
================<br>
Comment at: lib/sanitizer_common/tests/sanitizer_allocator_test.cc:650<br>
@@ +649,3 @@<br>
<div class="im">+  a.ForceLock();<br>
+  a.ForEachChunk<IterationTestCallback>(callback);<br>
+  a.ForceUnlock();<br>
</div>----------------<br>
Also here you can probably drop the explicit template argument.<br>
<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D539" target="_blank">http://llvm-reviews.chandlerc.com/D539</a><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div>
</div></div>