Jeff King wrote: > On Wed, Sep 13, 2017 at 11:24:31AM -0700, Jonathan Nieder wrote:
>> Compilers' signed/unsigned comparison warning can be noisy, but I'm >> starting to feel it's worth the suppression noise to turn it on when >> DEVELOPER=1 anyway. What do you think? Is there a way to turn it on >> selectively for certain functions on the LHS (like read() and write() >> style functions)? > > Obviously we couldn't turn them on for -Werror at this point. Let me see > how bad "-Wsign-compare -Wno-error=sign-compare" is. > > $ make 2>&1 | grep -c warning: > 1391 > > Ouch. I'm afraid that's probably not going to be very helpful. Even if I > were introducing a new problem, I'm not likely to see it amidst the mass > of existing complaints. Yup. Something like http://coccinelle.lip6.fr/rules/find_unsigned.html (but not that one precisely, since it's not smart enough to follow typedefs like size_t!) can do this for specific functions. Then "make coccicheck" would report new problematic usages. Let me know if this seems worth pursuing, and I'll send a patch based on the spatch I used to review your patch 8. Using coccinelle for this is a kind of oversized hammer. I wonder if e.g. sparse would be able to do a better job. > AFAIK there's no way to turn it on for specific functions, but I'm far > from a gcc-warning guru. Even if you could, though, the error may be far > from the function (e.g., if we store the result in an ssize_t and then > compare that with a size_t). If we have a check that catches the obvious cases then I'm already pretty happy. Though checking ssize_t vs size_t comparisons in general might also not be a bad idea, since it would be narrower than the general -Wsign-compare check. Thanks, Jonathan
