[patch] Warn on memcmp(a, b, sizeof(a) != 0) & co

[Alp Toker]

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 <thakis at chromium.org 
> <mailto:thakis at chromium.org>> 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
>     cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>     http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-- 
http://www.nuanti.com
the browser experts