Re: [PATCH v2 2/4] string-list: replace negative index encoding with "exact_match" parameter

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 23, 2025, 18:48 UTC
Message-ID: <xmqq348dovi3.fsf@gitster.g>
In-Reply-To: <CAOLa=ZShms1D-cq=x04dtT2ULTVE3ZDo8DODFnJRP2wcJz0EgQ@mail.gmail.com>

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

Show 17 quoted lines

> shejialuo <shejialuo@gmail.com> writes:
>
>> We would return negative index to indicate exact match by converting the
>> original positive index to be "-1 - index" in
>> "string_list_find_insert_index", which requires callers to decode this
>> information. This approach has several limitations:
>>
>
> Nit: It would be nice to start by explaining what
> "string_list_find_insert_index" does and then talking about the negative
> index. Perhaps something like:
>
>   The `string_list_find_insert_index()` function is used to determine
>   the correct insertion index for a new string within the string list.
>   The function also doubles up to convey if the string is already
>   existing in the list, this is done by returning a negative index
>   "-1 -index". Users are expected to decode this information.

Yeah, such an introductory statement would help those who are not familiar with the convention. Thanks for suggesting it.

>> 1. It prevents us from using the full range of size_t, which is
>>    necessary for large string list.

It is a disease to think that countable things must be counted in size_t and it needs to be somehow cured.

It is a type to count the size of memory allocations, nothing more. If you are holding 1000-bytes per the stuff you are counting, you would not need the full range of size_t --- you'll ran out your memory way before you fill size_t with the things you are counting.

When there is no external constraints (like you need to specify exact size to describe a file format to be interoperable), the most appropriate type to count things in is a platform natural "int". You wouldn't be handling billions of strings in string-list anyway (and that is smaller than half of 32-bit size_t; 64-bit size_t is much larger).

>> 2. Using int for indices while other parts of the codebase use size_t
>>    creates signed comparison warnings when these values are compared.

The other thing may be (mis)using size_t when it should not be. If they were also using "int" that would also squelch the warnings from "-Wsign-compare".

For an amusing read:

  https://lore.kernel.org/lkml/CAHk-=wg+_6eQnLWm-kihFxJo1_EmyLSGruKVGzuRUwACE=osrA@mail.gmail.com/