Re: [PATCH v2 3/4] string-list: change "string_list_find_insert_index" return type to "size_t"
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Sep 23, 2025, 09:44 UTC
- Message-ID
- <CAOLa=ZSGb_vvcneJ-b7ahL8J46a-FFKzupXaO0eKcyNehwTF+w@mail.gmail.com>
- In-Reply-To
- <aMp9Qs2AHBax62LL@ArchLinux>
shejialuo <shejialuo@gmail.com> writes:
Show 60 quoted lines
> As "string_list_find_insert_index" is a simple wrapper of
> "get_entry_index" and the return type of "get_entry_index" is already
> "size_t", we could simply change its return type to "size_t".
>
> Update all callers to use size_t variables for storing the return value.
> The tricky fix is the loop condition in "mailmap.c" to properly handle
> "size_t" underflow by changing from `0 <= --i` to `i--`.
>
> Remove "DISABLE_SIGN_COMPARE_WARNINGS" from "mailmap.c" as it's no
> longer needed with the proper unsigned types.
>
> Signed-off-by: shejialuo <shejialuo@gmail.com>
> ---
> add-interactive.c | 2 +-
> mailmap.c | 5 ++---
> refs.c | 4 +---
> string-list.c | 4 ++--
> string-list.h | 4 ++--
> 5 files changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/add-interactive.c b/add-interactive.c
> index 7c0fd3d218..19def3168a 100644
> --- a/add-interactive.c
> +++ b/add-interactive.c
> @@ -222,7 +222,7 @@ static void find_unique_prefixes(struct prefix_item_list *list)
> static ssize_t find_unique(const char *string, struct prefix_item_list *list)
> {
> bool exact_match;
> - int index = string_list_find_insert_index(&list->sorted, string, &exact_match);
> + size_t index = string_list_find_insert_index(&list->sorted, string, &exact_match);
> struct string_list_item *item;
>
> if (list->items.nr != list->sorted.nr)
> diff --git a/mailmap.c b/mailmap.c
> index 58a4484963..37fd158a51 100644
> --- a/mailmap.c
> +++ b/mailmap.c
> @@ -1,5 +1,4 @@
> #define USE_THE_REPOSITORY_VARIABLE
> -#define DISABLE_SIGN_COMPARE_WARNINGS
>
> #include "git-compat-util.h"
> #include "environment.h"
> @@ -244,7 +243,7 @@ static struct string_list_item *lookup_prefix(struct string_list *map,
> const char *string, size_t len)
> {
> bool exact_match;
> - int i = string_list_find_insert_index(map, string, &exact_match);
> + size_t i = string_list_find_insert_index(map, string, &exact_match);
> if (exact_match) {
> if (!string[len])
> return &map->items[i];
> @@ -266,7 +265,7 @@ static struct string_list_item *lookup_prefix(struct string_list *map,
> * overlong key would be inserted, which must come after the
> * real location of the key if one exists.
> */
> - while (0 <= --i && i < map->nr) {
> + while (i-- && i < map->nr) {
> int cmp = strncasecmp(map->items[i].string, string, len);
> if (cmp < 0)So earlier, if `i = 0`, we'd have a negative number and then the loop would exit. But with `size_t`, i can never be negative. So by using `i--`, we exit if `i = 0`. Okay
Show 51 quoted lines
> /*
> diff --git a/refs.c b/refs.c
> index f1ff5bf846..a8f06b9a0a 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -1688,8 +1688,6 @@ const char *find_descendant_ref(const char *dirname,
> const struct string_list *extras,
> const struct string_list *skip)
> {
> - int pos;
> -
> if (!extras)
> return NULL;
>
> @@ -1699,7 +1697,7 @@ const char *find_descendant_ref(const char *dirname,
> * with dirname (remember, dirname includes the trailing
> * slash) and is not in skip, then we have a conflict.
> */
> - for (pos = string_list_find_insert_index(extras, dirname, NULL);
> + for (size_t pos = string_list_find_insert_index(extras, dirname, NULL);
> pos < extras->nr; pos++) {
> const char *extra_refname = extras->items[pos].string;
>
> diff --git a/string-list.c b/string-list.c
> index c589ab5a2c..08dc00984c 100644
> --- a/string-list.c
> +++ b/string-list.c
> @@ -91,8 +91,8 @@ bool string_list_has_string(const struct string_list *list, const char *string)
> return exact_match;
> }
>
> -int string_list_find_insert_index(const struct string_list *list, const char *string,
> - bool *exact_match)
> +size_t string_list_find_insert_index(const struct string_list *list, const char *string,
> + bool *exact_match)
> {
> return get_entry_index(list, string, exact_match);
> }
> diff --git a/string-list.h b/string-list.h
> index 8830ce671d..6b0a2f4752 100644
> --- a/string-list.h
> +++ b/string-list.h
> @@ -173,8 +173,8 @@ void string_list_remove_empty_items(struct string_list *list, int free_util);
>
> /** Determine if the string_list has a given string or not. */
> bool 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,
> - bool *exact_match);
> +size_t string_list_find_insert_index(const struct string_list *list, const char *string,
> + bool *exact_match);
>Super nit: can we also add documentation to this function while we're here.
> /** > * Insert a new element to the string_list. The returned pointer can > -- > 2.51.0
The patch looks good to me.