Re: [Newcomer PATCH] log-tree.c: Supress Wsign-compare-warning

From: Patrick Steinhardt <ps@pks.im>
Date: May 23, 2025, 15:12 UTC
Message-ID: <aDCQWr3MBX4L7sbA@pks.im>
In-Reply-To: <xmqqsekx8yef.fsf@gitster.g>

On Wed, May 21, 2025 at 02:08:40PM -0700, Junio C Hamano wrote:

Show 7 quoted lines

> Fernando Gouveia Lima fernandolimabusiness@gmail.com writes:
> Quite honestly, -Wsign-compare is mostly garbage [*] and I wish we
> did not add it to the developer settings.  A more effective way to
> squelch them is not by sprinkling the casts like this, but to remove
> it from config.mak.dev ;-)
> 
> https://staticthinking.wordpress.com/2023/07/25/wsign-compare-is-garbage/

I'm still not of the opinion that it is garbage. We have tons of locations where we mismatch integer types only because we never got a warning from the compiler, and these have caused multiple stack overflows in the past. The signal-to-noise ratio is high, that much is certainly true. But if it helps us to avoid security issues in the future I think that is acceptable.

I do agree though this not a good project for newcomers, as fixing those bugs is quite intricate overall. So we should definitely remove this project from the microprojects page.

And in case I'm the only one who thinks that the warning has merit I'm also happy to be overruled and have it be removed from our developer settings.

Patrick