From: Jeff King <peff@peff.net>
Date: Fri, 15 Sep 2017 00:37:54 GMT
Subject: Re: [PATCH v2] config: avoid "write_in_full(fd, buf, len) < len" pattern
Message-ID: <20170915003754.wlckhuwcftyc32de@sigill.intra.peff.net>
In-Reply-To: <1012b31b-7bdd-ea20-f004-1c9f80de733c@ramsayjones.plus.com>

On Thu, Sep 14, 2017 at 12:31:38AM +0100, Ramsay Jones wrote:

> > Hmm, about three or four years ago, I spent two or three evenings
> > getting git to compile -Wextra clean. I remember the signed/unsigned
> > issue was the cause of a large portion of the warnings issued by
> > the compiler. I was surprised that it took such a relatively short
> > time to do. However, it affected quite a large portion of the code, so
> > I didn't think Junio would even consider applying it. Also, I only used
> > gcc and was anticipating having additional warnings on clang (but I
> > didn't get around to trying).
> > 
> > Maybe I should give it another go. :-D
> 
> For example, I remember the patch given below reduced the number
> of warnings quite a bit (because it's an inline function in a
> header file).
> 
> I just tried it again tonight; the current master branch has 3532
> warnings when compiled with -Wextra, 1409 of which are -Wsign-compare
> warnings. After applying the patch below, those numbers are reduced
> by 344 to 3188/1065.

I'd love it if we were clean on -Wextra. My big question is many
contortions we'd have to go through in the code. I don't mind at all if
we're actually cleaning as we go (e.g., using types of the correct
signedness, or preventing possible funny interactions). I'm just worried
it will turn into a bunch of noisy casts.

The patch you showed seems like an improvement to the code, but I don't
know if that would be the case for all of them. :)

-Peff