git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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.
Previous: shejialuoNext: shejialuo
Message 31 of 43 in “enhance string-list API to fix sign compare warnings”
  1. 0/4 enhance string-list API to fix sign compare warningsshejialuo, Sep 7, 2025
  2. 1/4 string-list: allow passing NULL for `get_entry_index`shejialuo, Sep 7, 2025
  3. Patrick SteinhardtSep 9, 2025
  4. 2/4 string-list: replace negative index encoding with "exact_match" parametershejialuo, Sep 7, 2025
  5. Patrick SteinhardtSep 9, 2025
  6. shejialuoSep 15, 2025
  7. 3/4 string-list: change "string_list_find_insert_index" return type to "size_t"shejialuo, Sep 7, 2025
  8. Patrick SteinhardtSep 9, 2025
  9. Junio C HamanoSep 9, 2025
  10. Patrick SteinhardtSep 10, 2025
  11. 4/4 refs: enable sign compare warnings checkshejialuo, Sep 7, 2025
  12. Patrick SteinhardtSep 9, 2025
  13. shejialuoSep 7, 2025
  14. 0/4 enhance string-list API to fix sign compare warningsshejialuo, Sep 17, 2025
  15. 1/4 string-list: use bool instead of int for "exact_match"shejialuo, Sep 17, 2025
  16. 2/4 string-list: replace negative index encoding with "exact_match" parametershejialuo, Sep 17, 2025
  17. Patrick SteinhardtSep 23, 2025
  18. shejialuoOct 5, 2025
  19. Karthik NayakSep 23, 2025
  20. Junio C HamanoSep 23, 2025
  21. Jeff KingSep 24, 2025
  22. Junio C HamanoSep 24, 2025
  23. Jeff KingSep 25, 2025
  24. Junio C HamanoSep 25, 2025
  25. Jeff KingOct 9, 2025
  26. Collin FunkOct 8, 2025
  27. Jeff KingOct 9, 2025
  28. shejialuoOct 5, 2025
  29. shejialuoOct 5, 2025
  30. 3/4 string-list: change "string_list_find_insert_index" return type to "size_t"shejialuo, Sep 17, 2025
  31. Karthik NayakSep 23, 2025
  32. shejialuoOct 5, 2025
  33. 4/4 refs: enable sign compare warnings checkshejialuo, Sep 17, 2025
  34. 0/4 enhance string-list API to fix sign compare warningsshejialuo, Oct 6, 2025
  35. 1/4 string-list: use bool instead of int for "exact_match"shejialuo, Oct 6, 2025
  36. 2/4 string-list: replace negative index encoding with "exact_match" parametershejialuo, Oct 6, 2025
  37. 3/4 string-list: change "string_list_find_insert_index" return type to "size_t"shejialuo, Oct 6, 2025
  38. Jeff KingOct 9, 2025
  39. 4/4 refs: enable sign compare warnings checkshejialuo, Oct 6, 2025
  40. Junio C HamanoOct 6, 2025
  41. Collin FunkOct 8, 2025
  42. Junio C HamanoOct 8, 2025
  43. Karthik NayakOct 8, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.