Re: [GSoC PATCH] rm: fix sign comparison warnings
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 13, 2025, 15:25 UTC
- Message-ID
- <xmqqmsdpq7ff.fsf@gitster.g>
- In-Reply-To
- <CAOLa=ZTia95Lib6bkz_nWi2BYEteAaOxsrrX9DqLTEz1t02ggA@mail.gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 41 quoted lines
> 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.