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
