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

[Patrick Steinhardt <ps@pks.im>]

On Fri, Jan 17, 2025 at 04:49:09PM +0900, Mike Hommey wrote:
> 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.

> @@ -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