<div dir="ltr">r198063, thanks!</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Dec 26, 2013 at 12:19 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.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"><br>
On 26/12/2013 20:12, Reid Kleckner wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Nice! LGTM. Looking forward to rolling this out over more code. :)<br>
</blockquote>
<br></div>
LGTM too. We had a discussion about this off-list with Nico just before the holiday and he made a good case for it following my initial review but I've just got round to following up.<br>
<br>
If we want to tweak the FixIt we can do that separately.<br>
<br>
Thanks for holding on Nico :-)<br>
<br>
Alp.<br>
<br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
<br>
<br>
On Fri, Dec 20, 2013 at 11:31 PM, Nico Weber <<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a> <mailto:<a href="mailto:thakis@chromium.org" target="_blank">thakis@chromium.org</a>>> wrote:<br>
<br>
Hi,<br>
<br>
the attached patch adds a new warning that makes memcmp & co warn<br>
on misplaced parentheses such as<br>
<br>
if (memcmp(a, b, sizeof(a) != 0))<br>
<br>
like so:<br>
<br>
test4.cc:5:30: warning: size argument in 'memcmp' call is a<br>
comparison [-Wmemsize-comparison]<br>
if (memcmp(a, b, sizeof(a) != 0))<br>
~~~~~~~~~~^~~~<br>
test4.cc:5:7: note: did you mean to compare the result of 'memcmp'<br>
instead?<br>
if (memcmp(a, b, sizeof(a) != 0))<br>
^ ~<br>
)<br>
test4.cc:5:20: note: explicitly cast the argument to size_t to<br>
silence this warning<br>
if (memcmp(a, b, sizeof(a) != 0))<br>
^<br>
(size_t)( )<br>
1 warning generated.<br>
<br>
<br>
This would have found one bug in NSS that we recently fixed [1]<br>
and found one more bug in chromium we didn't know about before<br>
[2]. It had 0 false positives on all of chromium.<br>
<br>
The idea of triggering this warning on a binop in the size<br>
argument is due to rnk.<br>
<br>
This warning can possibly be extended later on, but I feel this is<br>
a good start.<br>
<br>
Thoughts?<br>
<br>
Nico<br>
<br>
[1]:<br>
<a href="https://codereview.chromium.org/99423002/diff/1/net/third_party/nss/ssl/ssl3con.c" target="_blank">https://codereview.chromium.<u></u>org/99423002/diff/1/net/third_<u></u>party/nss/ssl/ssl3con.c</a><br>
[2]: <a href="https://codereview.chromium.org/8431007/#msg12" target="_blank">https://codereview.chromium.<u></u>org/8431007/#msg12</a><br>
<br>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br></div></div>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a> <mailto:<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.<u></u>edu</a>><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><div class="im"><br>
<br>
<br>
<br>
<br>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><br>
</div></blockquote>
<br><div class="HOEnZb"><div class="h5">
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</div></div></blockquote></div><br></div>