Subject: Re: [PATCH 7/9] builtin/log: fix remaining -Wsign-compare warnings

From: shejialuo <[email-masked]>
Date: Fri, 27 Dec 2024 21:21:56 +0800

On Fri, Dec 27, 2024 at 11:46:27AM +0100, Patrick Steinhardt wrote:

[snip]

> @@ -717,14 +715,14 @@ static int show_tag_object(const struct object_id *oid, struct rev_info *rev)
>  	unsigned long size;
>  	enum object_type type;
>  	char *buf = repo_read_object_file(the_repository, oid, &type, &size);
> -	int offset = 0;
> +	unsigned long offset = 0;

Why here we use `unsigned long`, is this a special situation where we
cannot use `size_t`?

>  
>  	if (!buf)
>  		return error(_("could not read object %s"), oid_to_hex(oid));
>  
>  	assert(type == OBJ_TAG);
>  	while (offset < size && buf[offset] != '\n') {
> -		int new_offset = offset + 1;
> +		unsigned long new_offset = offset + 1;
>  		const char *ident;
>  		while (new_offset < size && buf[new_offset++] != '\n')
>  			; /* do nothing */

> @@ -2183,7 +2182,7 @@ int cmd_format_patch(int argc,
>  		fmt_patch_suffix = cfg.fmt_patch_suffix;
>  
>  	/* Make sure "0000-$sub.patch" gives non-negative length for $sub */
> -	if (cfg.log.fmt_patch_name_max <= strlen("0000-") + strlen(fmt_patch_suffix))
> +	if (cfg.log.fmt_patch_name_max <= cast_size_t_to_int(strlen("0000-") + strlen(fmt_patch_suffix)))

A design question, why we don't change the type of
`cfg.log.fmt_patch_name_max` to be `size_t`?

>  		cfg.log.fmt_patch_name_max = strlen("0000-") + strlen(fmt_patch_suffix);
>  
>  	if (cover_from_description_arg)
> 
> -- 
> 2.48.0.rc0.184.g0fc57dec57.dirty
> 

Thanks,
Jialuo