First off, thanks for the review.<br><br><div class="gmail_quote">On Wed, Jun 15, 2011 at 7:27 PM, Nico Weber <span dir="ltr"><<a href="mailto:thakis@chromium.org">thakis@chromium.org</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">> Nico, please review; does this miss any of the bugs you were trying to<br>
> find with this warning? The array test case you had should be caught by<br>
> the array-specific sizeof warning I think.<br>
<br>
</div>Yes, now it doesn't find <a href="http://codereview.chromium.org/7167009/" target="_blank" class="cremed">http://codereview.chromium.org/7167009/</a><br>
<br>
Can you provide any examples where this warns when it shouldn't? (…and<br>
maybe revert this in the meantime?)</blockquote></div><br><div>Ugh. So, I'd really rather not revert this, as it hits a frustratingly common false positive I added to the test cases: copying a pointer's value into or out of a character array, often from an opaque memory buffer. If the pointer is itself a character pointer, then the warning fires. Because the standard specifically blesses the use af character arrays and memcpy, it seems bad to warn on their interactions just because of one particular sizeof() usage in the third argument.</div>
<div><br></div><div>Looking at the bug you point to, it's not clear why comparing the types of the first argument and the sizeof argument is the principled way to warn about this construct. You're not copying characters, and so the expected type of the argument to sizeof() isn't a 'char' (as the warning suggests). Instead, the correct code doesn't have any sizeof at all...</div>
<div><br></div><div>Could chat and see if there is some better way to flag this kind of code?</div>