Re: [PATCH v2 3/8] string-list: return index directly when inserting an existing element

[Jeff King <peff@peff.net>]

On Sun, May 18, 2025 at 11:57:15PM +0800, shejialuo wrote:

> When inserting an existing element, "add_entry" would convert "index"
> value to "-1-index" to indicate the caller that this element is in the
> list already.
> 
> However, in "string_list_insert", we would simply convert this to the
> original positive index without any further action. Let's directly
> return the index as we don't care about whether the element is in the
> list by using "add_entry".
> 
> In the future, if we want to let "add_entry" tell the caller, we may add
> "int *exact_match" parameter to "add_entry" instead of converting the
> index to negative to indicate.

I assumed this was in the same boat as the previous change: something we
used to use and now don't. But I don't think we ever did. The "-1-index"
pattern goes all the way back to the beginning of the code.

It does match how other functions like string_list_find_insert_index()
behave. But I think that pattern doesn't make much sense for
add_entry(). After the function returns we know we've either found
something or added it, so the positive index will always point to a
matching entry.

So I think your patches are correct, but I was curious how we got to
this state.

-Peff