<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Apr 23, 2014 at 2:38 PM, Bernhard Reutner-Fischer <span dir="ltr"><<a href="mailto:rep.dot.nop@gmail.com" target="_blank">rep.dot.nop@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="">On 23 April 2014 10:57, Kostya Serebryany <<a href="mailto:kcc@google.com">kcc@google.com</a>> wrote:<br>

> The patch is huge and has too many ifdefs.<br>
> It will have some time for us to review and will be very costly to maintain.<br>
<br>
</div>Why do you think it will be very costly to maintain?<br></blockquote><div><br></div><div>My assertion is based on experience. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

These things (e.g. glob_t members) do not change between glibc-releases, so<br>
i fail to see how this would be costly to maintain? It's pretty much a one-time<br>
thing to do IMO.<br>
<div class=""><br>
> So, before we continue with review I would like to hear the motivation and<br>
<br>
</div>The motivation behind this patch is that there are libc's apart from glibc, and<br>
these libc implementations, being POSIX compliant, may lack certain GNU<br>
extensions (as implemented in glibc). As you can see in your own code, you<br>
have gazillions of conditionals (preprocessor conditionals, which i add just add<br>
a couple of more, generic ones, btw, to answer your comment from above) to<br>
handle these other libc's.<br></blockquote><div><br></div><div>Ok. What particular libc implementation are you trying to use? </div><div>And what particular tool are you trying to use (asan, tsan, msan)?</div><div>Most of the things you hide under ifdefs are important only/mostly for msan, which does not exist in the GCC variant anyway. </div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
The route this patch takes it to probe these non-standard things and<br>
act accordingly.<br>
<div class=""><br>
> whether you are welling to help us maintain this code in future (= set up a public build bot)<br>
<br>
</div>I'm certainly willing to help maintain this code in the future since i<br>
want to use<br>
your nice sanitizers via gcc, sure. I cannot, however, setup a public<br>
build-bot since<br>
my notebook is not prepared to handle this task on a regular basis.<br>
If you break stuff upstream and i find the backported gcc version to<br>
be dysfunctional, i<br>
will send you fixes if time permits.<br></blockquote><div><br></div><div>This round trip has very long latency and brings heavy burden on us (code maintainers).</div><div>We are very reluctant to pay this price. </div><div>
Having a public bot for llvm upstream code will reduce this cost.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

<br>
I think the patch itself is pretty straight forward, </blockquote><div><br></div><div>It is straight forward, but it is also huge. </div><div>Depending on your actual needs (see above) we might be able to do something *much* simpler. <br>
</div><div><br></div><div>--kcc </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">please let me<br>

know if you have specific<br>
questions about a technical aspect of the patch.<br>
<br>
Please apply.<br>
thanks,<br>
><br>
> <a href="http://reviews.llvm.org/D3464" target="_blank">http://reviews.llvm.org/D3464</a><br>
><br>
><br>
</blockquote></div><br></div></div>