From: Patrick Steinhardt <ps@pks.im>
Date: Tue, 09 Sep 2025 06:22:56 GMT
Subject: Re: [PATCH 2/4] string-list: replace negative index encoding with "exact_match" parameter
Message-ID: <aL_HwMEJBLjrV8WP@pks.im>
In-Reply-To: <aL219d7FYJQNKBEf@ArchLinux>

On Mon, Sep 08, 2025 at 12:42:29AM +0800, shejialuo wrote:
> diff --git a/mailmap.c b/mailmap.c
> index 56c72102d9..253517cdf6 100644
> --- a/mailmap.c
> +++ b/mailmap.c
> @@ -243,10 +243,9 @@ void clear_mailmap(struct string_list *map)
>  static struct string_list_item *lookup_prefix(struct string_list *map,
>  					      const char *string, size_t len)
>  {
> -	int i = string_list_find_insert_index(map, string, 1);
> -	if (i < 0) {
> -		/* exact match */
> -		i = -1 - i;
> +	int exact_match;
> +	int i = string_list_find_insert_index(map, string, &exact_match);
> +	if (exact_match) {
>  		if (!string[len])
>  			return &map->items[i];
>  		/*

Yeah, this looks much cleaner compared to before.

> diff --git a/string-list.c b/string-list.c
> index bf358d1a5c..224bc182ff 100644
> --- a/string-list.c
> +++ b/string-list.c
> @@ -92,13 +92,9 @@ int string_list_has_string(const struct string_list *list, const char *string)
>  }
>  
>  int string_list_find_insert_index(const struct string_list *list, const char *string,
> -				  int negative_existing_index)
> +				  int *exact_match)
>  {
> -	int exact_match;
> -	int index = get_entry_index(list, string, &exact_match);
> -	if (exact_match)
> -		index = -1 - (negative_existing_index ? index : 0);
> -	return index;
> +	return get_entry_index(list, string, exact_match);
>  }
>  
>  struct string_list_item *string_list_lookup(struct string_list *list, const char *string)

Okay, this here is where the preceding patch comes from, as some callers
pass `NULL` to `string_list_find_insert_index()`.

> diff --git a/string-list.h b/string-list.h
> index 2b438c7733..03c7009472 100644
> --- a/string-list.h
> +++ b/string-list.h
> @@ -174,7 +174,7 @@ void string_list_remove_empty_items(struct string_list *list, int free_util);
>  /** Determine if the string_list has a given string or not. */
>  int string_list_has_string(const struct string_list *list, const char *string);
>  int string_list_find_insert_index(const struct string_list *list, const char *string,
> -				  int negative_existing_index);
> +				  int *exact_match);
>  

Makes me wonder whether we want to use `bool *exact_match` now to hint
that this is really only a true/false value? If so, we'd also have to
adapt the signature in the preceding commit.

Patrick