Re: [PATCH] connect: address -Wsign-compare warnings

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 17, 2025, 17:26 UTC
Message-ID: <xmqqo705mjw7.fsf@gitster.g>
In-Reply-To: <20250117074909.1430067-1-mh@glandium.org>

Mike Hommey <mh@glandium.org> writes:

Show 7 quoted lines

> Most of the warnings were about loop variables being declared as ints
> with a condition using a size_t, whereby switching the variable to
> size_t fixes the warning.
>
> One other case was comparing the result of strlen to an int passed
> as an argument, which turns out could just as well be passed as a
> size_t, albeit trickling to other functions.

As long as the blast radius is limited (like this one, which most of the cascades were within the callchain of file-scope statics), and the changes of type is going in the right direction (in this case, I see all are using size_t for length that may come from or compared with the result of strlen(), which falls into that category), such a change is very much welcomed.

Even if the primary objective is to squelch the -Wsign-compare and even if we are talking about a line in packet_reader object, which would not exceed 64k bytes and using size_t is way overkill, that is. I personally do not think -Wsign-compare cleanliness is buying us all that much, compared to the amount of code churn. But this one is well within the level that I can tolerate ;-).

Will queue.  Thanks.