Please hold on with this patch. <div>I'd really like to understand the consequences but have not time until late next week (traveling, etc). </div><div>It would help if you could make a test for this new functionality. </div>
<div><br></div><div>--kcc <br><br><div class="gmail_quote">On Thu, Apr 12, 2012 at 7:22 AM,  <span dir="ltr"><<a href="mailto:samsonov@google.com">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">
<div class="im">On 2012/04/12 14:21:26, samsonov wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<a href="http://codereview.appspot.com/6010049/diff/1/interception.h" target="_blank">http://codereview.appspot.com/<u></u>6010049/diff/1/interception.h</a><br>
File interception.h (right):<br>
</blockquote>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<a href="http://codereview.appspot.com/6010049/diff/1/interception.h#newcode78" target="_blank">http://codereview.appspot.com/<u></u>6010049/diff/1/interception.h#<u></u>newcode78</a><br>
interception.h:78: # define DECLARE_WRAPPER(ret_type, convention,<br>
</blockquote>
func, ...)<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Naming was not self-explanatory already and now it's even worse :( As<br>
</blockquote>
I get it,<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
you use DECLARE_WRAPPER(...func...) to define "func" that is:<br>
1) alias to an interceptor if instrumented code doesn't define its own<br>
</blockquote>
"func"<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2) breaks linkage if instrumented code does define its own "func"<br>
Why not call this macro smth like<br>
</blockquote>
"ADD_GUARD_AGAINST_CUSTOM_<u></u>DEFINITION" or smth<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
like that.<br>
</blockquote>
<br></div>
I mean, "breaks in runtime"<div class="HOEnZb"><div class="h5"><br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<a href="http://codereview.appspot.com/6010049/diff/1/interception.h#newcode91" target="_blank">http://codereview.appspot.com/<u></u>6010049/diff/1/interception.h#<u></u>newcode91</a><br>
interception.h:91: # define WRAP(x) interception_wrap_##x<br>
This fails on our output tests: the top stack frame of allocation now<br>
</blockquote>
says<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
"interception_wrap_malloc" instead of "malloc". Can we use some other<br>
</blockquote>
naming?<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<a href="http://codereview.appspot.com/6010049/diff/1/interception.h#newcode96" target="_blank">http://codereview.appspot.com/<u></u>6010049/diff/1/interception.h#<u></u>newcode96</a><br>
interception.h:96: __attribute__((weak,<br>
</blockquote>
alias("interception_wrap_"#<u></u>func), \<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
alias(WRAPPER_NAME(func)) ?<br>
</blockquote>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<a href="http://codereview.appspot.com/6010049/diff/1/interception.h#newcode97" target="_blank">http://codereview.appspot.com/<u></u>6010049/diff/1/interception.h#<u></u>newcode97</a><br>
interception.h:97: visibility("default")))<br>
You can use INTERCEPTOR_ATTRIBUTE instead of visibility("default")<br>
</blockquote>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<a href="http://codereview.appspot.com/6010049/diff/1/interception_linux.h" target="_blank">http://codereview.appspot.com/<u></u>6010049/diff/1/interception_<u></u>linux.h</a><br>
File interception_linux.h (right):<br>
</blockquote>
<br>
<br>
<a href="http://codereview.appspot.com/6010049/diff/1/interception_linux.h#newcode32" target="_blank">http://codereview.appspot.com/<u></u>6010049/diff/1/interception_<u></u>linux.h#newcode32</a><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
interception_linux.h:32: (void*)&func, (void*)&WRAP(func))<br>
I think that<br>
&& (void*)&func == (void*)&WRAP(func)<br>
would be better.<br>
Anyway, it's worth explaining this magic in the comment here or in<br>
interception.h<br>
</blockquote>
<br>
<br>
<br>
<a href="http://codereview.appspot.com/6010049/" target="_blank">http://codereview.appspot.com/<u></u>6010049/</a><br>
</div></div></blockquote></div><br></div>