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

[Jonathan Nieder <jrnieder@gmail.com>]

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