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

In "add_entry", we accept "insert_at" parameter which must be either -1
(auto) or between 0 and `list->nr` inclusive. Any other value is
invalid. When caller specify any invalid "insert_at" value, we won't
check the range and move the element, which would definitely cause the
trouble.

However, we only use "add_entry" in "string_list_insert" function and we
always pass the "-1" for "insert_at" parameter. So, we never use this
parameter to insert element in a user specified position. Let's delete
this parameter. If there is any requirement later, we need to use a
better way to do this.

We can see from looking at the code that removing this will not change
the behavior. But that always makes me wonder why it was there in the
first place, and whether we might ever want it.

The answer in this case is that we used to have another function,
string_list_insert_at_index(), which used the extra insert_at parameter.
The idea being that you could call string_list_find_insert_index(),
decide whether there was something already there, and then insert
without repeating the binary search.

But you can see in callers like 63226218ba (mailmap: use higher level
string list functions, 2014-11-24) that this was not really that useful
(in that commit we just try to insert and check the util pointer to see
if we need to add the auxiliary structure).

So the function went away in f8c4ab611a (string_list: remove
string_list_insert_at_index() from its API, 2014-11-24), and I suspect
we won't need it again. (Also, I think these days we'd probably use a
strmap instead anyway).

-Peff