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

[Nico Weber]

r198063, thanks!


On Thu, Dec 26, 2013 at 12:19 PM, Alp Toker <alp at nuanti.com> wrote:

>
> 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131226/bfaf0f6f/attachment.html>