On Mon, May 19, 2025 at 03:58:13AM -0400, Jeff King wrote: > 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. It seems that we create this in a long time ago. In 8fd2cb4069 (Extract helper bits from c-merge-recursive work, 2006-07-25), we introduce the "path-list.c", at that time, we have the code already. > > -Peff Thanks, Jialuo
