Re: [PATCH v2] config: avoid "write_in_full(fd, buf, len) < len" pattern

Jeff King wrote:

On Wed, Sep 13, 2017 at 11:24:31AM -0700, Jonathan Nieder wrote:

quoted

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