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

From: Patrick Steinhardt <ps@pks.im>
Date: Jan 17, 2025, 09:36 UTC
Message-ID: <Z4okjR8YfUGvnt1t@pks.im>
In-Reply-To: <20250117074909.1430067-1-mh@glandium.org>

On Fri, Jan 17, 2025 at 04:49:09PM +0900, Mike Hommey wrote:

Show 13 quoted lines

> diff --git a/connect.c b/connect.c
> index 10fad43e98..91f3990014 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -77,7 +76,7 @@ static NORETURN void die_initial_contact(int unexpected)
>  /* Checks if the server supports the capability 'c' */
>  int server_supports_v2(const char *c)
>  {
> -	int i;
> +	size_t i;
>  
>  	for (i = 0; i < server_capabilities_v2.nr; i++) {
>  		const char *out;

I know that it's often frowned upon to change formatting while at it. But in the context of these refactorings I think that it's quite helpful if you also moved the loop index variable declarations into the loops themselves. This allows us to trivially see that it's not used anywhere else.

Show 15 quoted lines

> @@ -232,12 +231,12 @@ static void annotate_refs_with_symref_info(struct ref *ref)
>  	string_list_clear(&symref, 0);
>  }
>  
> -static void process_capabilities(struct packet_reader *reader, int *linelen)
> +static void process_capabilities(struct packet_reader *reader, size_t *linelen)
>  {
>  	const char *feat_val;
>  	size_t feat_len;
>  	const char *line = reader->line;
> -	int nul_location = strlen(line);
> +	size_t nul_location = strlen(line);
>  	if (nul_location == *linelen)
>  		return;
>  	server_capabilities_v1 = xstrdup(line + nul_location + 1);

I think splitting out the strlen(3p)-related changes into a separate commit might make sense.

Thanks for working on this, quite happy to see that this gets picked up by the community!

Patrick