Re: [GSoC PATCH] rm: fix sign comparison warnings

[Junio C Hamano <gitster@pobox.com>]

Karthik Nayak <karthik.188@gmail.com> writes:

> I have to agree. I think it would a bit cleaner to actually change the
> functions argument type itself. Perhaps, something like:
>
> -- >8 --
>
> diff --git a/builtin/rm.c b/builtin/rm.c
> index 12ae086a55..79e47d6e9e 100644
> --- a/builtin/rm.c
> +++ b/builtin/rm.c
> @@ -40,10 +40,8 @@ static struct {
>  	} *entry;
>  } list;
>
> -static int get_ours_cache_pos(const char *path, int pos)
> +static int get_ours_cache_pos(const char *path, unsigned int i)
>  {
> -	int i = -pos - 1;
> -
>  	while ((i < the_repository->index->cache_nr) &&
> !strcmp(the_repository->index->cache[i]->name, path)) {
>  		if (ce_stage(the_repository->index->cache[i]) == 2)
>  			return i;
> @@ -83,7 +81,7 @@ static void submodules_absorb_gitdir_if_needed(void)
>
>  		pos = index_name_pos(the_repository->index, name, strlen(name));
>  		if (pos < 0) {
> -			pos = get_ours_cache_pos(name, pos);
> +			pos = get_ours_cache_pos(name, -pos - 1);
>  			if (pos < 0)
>  				continue;
>  		}
>
> @@ -131,7 +129,7 @@ static int check_local_mod(struct object_id *head,
> int index_only)
>  			 * Skip unmerged entries except for populated submodules
>  			 * that could lose history when removed.
>  			 */
> -			pos = get_ours_cache_pos(name, pos);
> +			pos = get_ours_cache_pos(name, -pos - 1);
>  			if (pos < 0)
>  				continue;

It makes the code far easier to read to turn what index_name_pos()
returns to the index where the conflicted "our" entry ought to
appear like the above two hunks do.  Makes perfect sense, even if
there weren't any type mismatch warnings.

Thanks.