From: Junio C Hamano Date: Tue, 23 Sep 2025 18:48:36 GMT Subject: Re: [PATCH v2 2/4] string-list: replace negative index encoding with "exact_match" parameter Message-ID: In-Reply-To: Karthik Nayak writes: > shejialuo 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/