<div class="gmail_quote">On Thu, May 31, 2012 at 5:22 PM,  <span dir="ltr"><<a href="mailto:samsonov@google.com" target="_blank">samsonov@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Reviewers: <a href="http://llvm-commits_cs.uiuc.edu" target="_blank">llvm-commits_cs.uiuc.edu</a>, dvyukov,<div class="im"><br>
<br>
<br>
<a href="http://codereview.appspot.com/6258065/diff/1/lib/sanitizer_common/symbolizer.h" target="_blank">http://codereview.appspot.com/<u></u>6258065/diff/1/lib/sanitizer_<u></u>common/symbolizer.h</a><br>
File lib/sanitizer_common/<u></u>symbolizer.h (right):<br>
<br>
<a href="http://codereview.appspot.com/6258065/diff/1/lib/sanitizer_common/symbolizer.h#newcode1" target="_blank">http://codereview.appspot.com/<u></u>6258065/diff/1/lib/sanitizer_<u></u>common/symbolizer.h#newcode1</a><br>

lib/sanitizer_common/<u></u>symbolizer.h:1: //===-- symbolizer.h<br>
------------------------------<u></u>--------------*- C++ -*-===//<br></div><div class="im">
On 2012/05/31 12:24:58, dvyukov wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Let's prefix files in sanitizer_common with some prefixes.<br>
</blockquote></div>
Isn't directory name enough? What is your suggestion: sanitizer_? san_?</blockquote><div><br></div><div>Object files sometimes clash.</div><div>sanitizer_</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
<br>
<a href="http://codereview.appspot.com/6258065/diff/1/lib/sanitizer_common/symbolizer.h#newcode12" target="_blank">http://codereview.appspot.com/<u></u>6258065/diff/1/lib/sanitizer_<u></u>common/symbolizer.h#newcode12</a><br>

lib/sanitizer_common/<u></u>symbolizer.h:12: #ifndef SYMBOLIZER_H<br></div><div class="im">
On 2012/05/31 12:24:58, dvyukov wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I would put the prefix here as well.<br>
</blockquote></div>
ditto.<div class="im"><br>
<br>
<a href="http://codereview.appspot.com/6258065/diff/1/lib/sanitizer_common/symbolizer.h#newcode19" target="_blank">http://codereview.appspot.com/<u></u>6258065/diff/1/lib/sanitizer_<u></u>common/symbolizer.h#newcode19</a><br>

lib/sanitizer_common/<u></u>symbolizer.h:19: typedef unsigned long uptr;<br></div><div class="im">
On 2012/05/31 12:24:58, dvyukov wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Move this to another file.<br>
</blockquote>
<br></div>
Done.<div class="im"><br>
<br>
<a href="http://codereview.appspot.com/6258065/diff/1/lib/sanitizer_common/symbolizer.h#newcode23" target="_blank">http://codereview.appspot.com/<u></u>6258065/diff/1/lib/sanitizer_<u></u>common/symbolizer.h#newcode23</a><br>

lib/sanitizer_common/<u></u>symbolizer.h:23: char *module;<br></div><div class="im">
On 2012/05/31 12:24:58, dvyukov wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
What are these pointers? Who frees them? What is the lifetime?<br>
</blockquote></div>
On a second thought, I though that Symbolizer should better return a<br>
pointer to AddressInfo and transfer ownership.</blockquote><div><br></div><div>What about strings?</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">
<a href="http://codereview.appspot.com/6258065/diff/1/lib/sanitizer_common/symbolizer.h#newcode31" target="_blank">http://codereview.appspot.com/<u></u>6258065/diff/1/lib/sanitizer_<u></u>common/symbolizer.h#newcode31</a><br>

lib/sanitizer_common/<u></u>symbolizer.h:31: class Symbolizer {<br></div><div class="im">
On 2012/05/31 12:24:58, dvyukov wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Are you going to use pimpl?<br>
</blockquote></div>
Hm-m, I hoped to get away with OS-specific files with implementation<br>
details (as we do in ASan now).</blockquote><div><br></div><div>The header will have a lot of unnecessary details.</div><div>Actually, what you need is a single function SymbolizeCode(...). You don't even need Initialize(), it won't do anything useful and won't return a useful return value.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"><br>
<br>
<a href="http://codereview.appspot.com/6258065/diff/1/lib/sanitizer_common/symbolizer.h#newcode36" target="_blank">http://codereview.appspot.com/<u></u>6258065/diff/1/lib/sanitizer_<u></u>common/symbolizer.h#newcode36</a><br>

lib/sanitizer_common/<u></u>symbolizer.h:36: bool SymbolizeCode(uptr address,<br>
AddressInfo* info);<br></div><div class="im">
On 2012/05/31 12:24:58, dvyukov wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
It must return a set of AddressInfo's with inlining info.<br>
</blockquote></div>
Can a FIXME be fine for now?<br></blockquote><div><br></div><div>Let's include it into the interface, but leave implementation as FIXME.</div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Please review this at <a href="http://codereview.appspot.com/6258065/" target="_blank">http://codereview.appspot.com/<u></u>6258065/</a><br>
<br>
Affected files:<br>
  M     sanitizer_common/mini_libc.h<br>
  A     sanitizer_common/symbolizer.cc<br>
  A     sanitizer_common/symbolizer.h<br>
<br>
<br>
</blockquote></div><br>