On Wed, Jun 15, 2011 at 7:35 PM, Chandler Carruth <[email protected]> wrote: > First off, thanks for the review. > > On Wed, Jun 15, 2011 at 7:27 PM, Nico Weber <[email protected]> wrote: >> >> > Nico, please review; does this miss any of the bugs you were trying to >> > find with this warning? The array test case you had should be caught by >> > the array-specific sizeof warning I think. >> >> Yes, now it doesn't find http://codereview.chromium.org/7167009/ >> >> Can you provide any examples where this warns when it shouldn't? (…and >> maybe revert this in the meantime?) > > 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.
You added char c = 42; char* parr[5]; memcpy(&parr[3], &c, sizeof(&c)); Doesn't this copy the (typically) 4-byte pointer parr[3] into the 1-byte memory occupied by c? Shouldn't this be char* c; char* parr[5]; memcpy(&parr[3], &c, sizeof(c)); (In which case the old version wouldn't warn either.) /me feels blind > 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... > Could chat and see if there is some better way to flag this kind of code? _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
