On 21/12/2013 07:31, Nico Weber 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))
~~~~~~~~~~^~~~
Hi Nico,
Agree we'd do well to diagnose here, but it seems can get a lot more
mileage by simplifying and generalizing the check.
This doesn't appear to be a memsize-specific problem, nor is it
necessarily limited to binop argument expressions.
Surely it's rather the case that any implicit bool-to-size_t call
argument conversion is at best dubious, and most likely a bug?
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)( )
A cast is the wrong FixIt to suggest. "sizeof(a) ? 1 : 0" is closer to
the natural way to write this construct if it was indeed their true
intention.
However it's best not to generate a FixIt in this context given that we
have a high degree of confidence that the user's code is bogus,
especially since the FixIt hides away the bug into a form that's harder
to detect.
Alp.
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]
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
