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

From: Karthik Nayak <karthik.188@gmail.com>
Date: Mar 13, 2025, 15:50 UTC
Message-ID: <CAOLa=ZQWWh2952HDazcdYkDxBAeuCS+KWQk66zVGZyb+foE1WA@mail.gmail.com>
In-Reply-To: <xmqqcyelq714.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

Show 14 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Also a bigger question is, shouldn't the type of `pathspec.nr` and
>> 'istate.cache_nr' be the actual change required? Shouldn't they be set
>> to 'size_t'?
>
> Please do not blindly advocate the idea that size_t is always the
> right type for any countables.  It is not.
>
> Platform natural way to count things is either "unsigned int", if
> you are only counting, or "int", if you need to be able to signal an
> unusual state other than "here is now many we have in the set", like
> how index related functions uses (-pos-1) to signal a location in
> the same range with different meanings.

I agree there is more nuance here, and that is what I was trying to clarify too.

In this particular case, since `istate.cache_nr` is used for capturing the number of elements in `index_state.cache` and the fact that we also use ALLOC_GROW on it, would suggest that the type should be 'size_t'. But I don't know much about the usage of 'index_state' itself, so I'll take what you say.