On 26/12/2013 20:12, Reid Kleckner wrote:
Nice! LGTM. Looking forward to rolling this out over more code. :)
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.
If we want to tweak the FixIt we can do that separately. Thanks for holding on Nico :-) Alp.
On Fri, Dec 20, 2013 at 11:31 PM, Nico Weber <[email protected] <mailto:[email protected]>> wrote:Hi, the attached patch adds a new warning that makes memcmp & co warn on misplaced parentheses such as if (memcmp(a, b, sizeof(a) != 0)) like so: test4.cc:5:30: warning: size argument in 'memcmp' call is a comparison [-Wmemsize-comparison] if (memcmp(a, b, sizeof(a) != 0)) ~~~~~~~~~~^~~~ test4.cc:5:7: note: did you mean to compare the result of 'memcmp' instead? if (memcmp(a, b, sizeof(a) != 0)) ^ ~ ) test4.cc:5:20: note: explicitly cast the argument to size_t to silence this warning if (memcmp(a, b, sizeof(a) != 0)) ^ (size_t)( ) 1 warning generated. This would have found one bug in NSS that we recently fixed [1] and found one more bug in chromium we didn't know about before [2]. It had 0 false positives on all of chromium. The idea of triggering this warning on a binop in the size argument is due to rnk. This warning can possibly be extended later on, but I feel this is a good start. Thoughts? Nico [1]: https://codereview.chromium.org/99423002/diff/1/net/third_party/nss/ssl/ssl3con.c [2]: https://codereview.chromium.org/8431007/#msg12 _______________________________________________ cfe-commits mailing list [email protected] <mailto:[email protected]> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
-- http://www.nuanti.com the browser experts _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
