Thread (47 messages) 47 messages, 3 authors, 2025-07-04

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

From: Patrick Steinhardt <hidden>
Date: 2025-05-19 07:18:03

On Sun, May 18, 2025 at 11:57:15PM +0800, shejialuo wrote:
quoted hunk ↗ jump to hunk
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.

Signed-off-by: shejialuo <redacted>
---
 string-list.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/string-list.c b/string-list.c
index 8540c29bc9..171cef5dbb 100644
--- a/string-list.c
+++ b/string-list.c
@@ -40,14 +40,13 @@ static int get_entry_index(const struct string_list *list, const char *string,
 	return right;
 }
 
-/* returns -1-index if already exists */
 static int add_entry(struct string_list *list, const char *string)
 {
 	int exact_match = 0;
 	int index = get_entry_index(list, string, &exact_match);
 
 	if (exact_match)
-		return -1 - index;
+		return index;
 
 	ALLOC_GROW(list->items, list->nr+1, list->alloc);
 	if (index < list->nr)
Okay, let's assume that "index == 2" here and we have an exact match.
We'd thus return `-1 - 2 == -3`.
quoted hunk ↗ jump to hunk
@@ -65,9 +64,6 @@ struct string_list_item *string_list_insert(struct string_list *list, const char
 {
 	int index = add_entry(list, string);
 
-	if (index < 0)
-		index = -1 - index;
-
 	return list->items + index;
 }
So we'd now realize that `index < 0` and thus calculate `-1 - -3 == 2`,
which is the original index indeed. So this is a nice simplification
that retains the original behaviour indeed.

I think we could simplify the code even further by inlining
`get_entry_index()` now that `string_list_insert()` is a trivial wrapper
around it. But I'll leave it up to you whether we want to do it or not.

Patrick
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help