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

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

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

> 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/