[PATCH v2 0/4] enhance string-list API to fix sign compare warnings

[shejialuo <shejialuo@gmail.com>]

Hi All:

This is a small PATCH to enhance string-list API
"string_list_find_insert_index" which has introduced sign compare
warnings.

---

Changes since v1:

1. Create a new commit which aims at using `bool` for "exact_match"
   parameter.
2. Rebase the [PATCH 1/4] and [PATCH 2/4] into a single [PATCH v2 2/4]
   commit to avoid confusing the user with the motivation of the
   original commit [PATCH 1/4].
3. Enhance the comimt message of [PATCH 2/4] to improve the motivation.
4. Update "i-- > 0" to "i--" for [PATCH 3/4]

Thanks,
Jialuo

shejialuo (4):
  string-list: use bool instead of int for "exact_match"
  string-list: replace negative index encoding with "exact_match"
    parameter
  string-list: change "string_list_find_insert_index" return type to
    "size_t"
  refs: enable sign compare warnings check

 add-interactive.c |  7 ++++---
 mailmap.c         | 10 ++++------
 refs.c            | 13 ++++---------
 string-list.c     | 29 ++++++++++++++---------------
 string-list.h     |  6 +++---
 5 files changed, 29 insertions(+), 36 deletions(-)

Range-diff against v1:
1:  d3333b4ff6 ! 1:  c3786fa386 string-list: allow passing NULL for `get_entry_index`
    @@ Metadata
     Author: shejialuo <shejialuo@gmail.com>
     
      ## Commit message ##
    -    string-list: allow passing NULL for `get_entry_index`
    +    string-list: use bool instead of int for "exact_match"
     
    -    Callers of `get_entry_index()` are required to pass a non-NULL
    -    `exact_match` parameter to receive information about whether an exact
    -    match is found. However, in some cases, callers only need the index
    -    position.
    -
    -    Let's allow callers to pass NULL for the `exact_match` parameter
    -    when they don't need this information, reducing unnecessary variable
    -    declarations in calling code.
    +    The "exact_match" parameter in "get_entry_index" is used to indicate
    +    whether a string is found or not, which is fundamentally a true/false
    +    value. As we allow the use of bool, let's use bool instead of int to
    +    make the function more semantically clear.
     
         Signed-off-by: shejialuo <shejialuo@gmail.com>
     
      ## string-list.c ##
    +@@ string-list.c: void string_list_init_dup(struct string_list *list)
    + /* if there is no exact match, point to the index where the entry could be
    +  * inserted */
    + static size_t get_entry_index(const struct string_list *list, const char *string,
    +-			      int *exact_match)
    ++			      bool *exact_match)
    + {
    + 	size_t left = 0, right = list->nr;
    + 	compare_strings_fn cmp = list->cmp ? list->cmp : strcmp;
     @@ string-list.c: static size_t get_entry_index(const struct string_list *list, const char *string
      		else if (compare > 0)
      			left = middle + 1;
      		else {
     -			*exact_match = 1;
    -+			if (exact_match)
    -+				*exact_match = 1;
    ++			*exact_match = true;
      			return middle;
      		}
      	}
      
     -	*exact_match = 0;
    -+	if (exact_match)
    -+		*exact_match = 0;
    ++	*exact_match = false;
      	return right;
      }
      
    + static size_t add_entry(struct string_list *list, const char *string)
    + {
    +-	int exact_match = 0;
    ++	bool exact_match;
    + 	size_t index = get_entry_index(list, string, &exact_match);
    + 
    + 	if (exact_match)
    +@@ string-list.c: struct string_list_item *string_list_insert(struct string_list *list, const char
    + void string_list_remove(struct string_list *list, const char *string,
    + 			int free_util)
    + {
    +-	int exact_match;
    ++	bool exact_match;
    + 	int i = get_entry_index(list, string, &exact_match);
    + 
    + 	if (exact_match) {
    +@@ string-list.c: void string_list_remove(struct string_list *list, const char *string,
    + 	}
    + }
    + 
    +-int string_list_has_string(const struct string_list *list, const char *string)
    ++bool string_list_has_string(const struct string_list *list, const char *string)
    + {
    +-	int exact_match;
    ++	bool exact_match;
    + 	get_entry_index(list, string, &exact_match);
    + 	return exact_match;
    + }
    +@@ string-list.c: 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;
    ++	bool exact_match;
    + 	int index = get_entry_index(list, string, &exact_match);
    + 	if (exact_match)
    + 		index = -1 - (negative_existing_index ? index : 0);
    +@@ string-list.c: int string_list_find_insert_index(const struct string_list *list, const char *st
    + 
    + struct string_list_item *string_list_lookup(struct string_list *list, const char *string)
    + {
    +-	int exact_match, i = get_entry_index(list, string, &exact_match);
    ++	bool exact_match;
    ++	size_t i = get_entry_index(list, string, &exact_match);
    + 	if (!exact_match)
    + 		return NULL;
    + 	return list->items + i;
    +
    + ## string-list.h ##
    +@@ string-list.h: void string_list_remove_empty_items(struct string_list *list, int free_util);
    + /* Use these functions only on sorted lists: */
    + 
    + /** Determine if the string_list has a given string or not. */
    +-int string_list_has_string(const struct string_list *list, const char *string);
    ++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,
    + 				  int negative_existing_index);
    + 
2:  104c090d8d ! 2:  7ac8fd69c0 string-list: replace negative index encoding with "exact_match" parameter
    @@ Commit message
         We would return negative index to indicate exact match by converting the
         original positive index to be "-1 - index" in
         "string_list_find_insert_index", which requires callers to decode this
    -    information.
    +    information. This approach has several limitations:
     
    -    This is bad due to the following reasons:
    +    1. It prevents us from using the full range of size_t, which is
    +       necessary for large string list.
    +    2. Using int for indices while other parts of the codebase use size_t
    +       creates signed comparison warnings when these values are compared.
     
    -    1. The callers need to convert the negative index back to the original
    -       positive value, which requires the callers to understand the detail
    -       of the function.
    -    2. As we have to return negative index, we need to specify the return
    -       type to be `int` instead of `size_t`, which would cause sign compare
    -       warnings.
    +    To address these limitations, change the function to return size_t for
    +    the index value and use a separate bool parameter to indicate whether
    +    the index refers to an existing entry or an insertion point.
     
    -    Refactor "string_list_find_insert_index" to use an output parameter
    -    "exact_match" for indicating the exact match rather than encoding
    -    through negative return values.
    +    In some cases, the callers of "string_list_find_insert_index" only need
    +    the index position and don't care whether an exact match is found.
    +    However, "get_entry_index" currently requires a non-NULL "exact_match"
    +    parameter, forcing these callers to declare unnecessary variables.
    +    Let's allow callers to pass NULL for the "exact_match" parameter when
    +    they don't need this information, reducing unnecessary variable
    +    declarations in calling code.
     
         Signed-off-by: shejialuo <shejialuo@gmail.com>
     
    @@ add-interactive.c: static void find_unique_prefixes(struct prefix_item_list *lis
      static ssize_t find_unique(const char *string, struct prefix_item_list *list)
      {
     -	int index = string_list_find_insert_index(&list->sorted, string, 1);
    -+	int exact_match;
    ++	bool exact_match;
     +	int index = string_list_find_insert_index(&list->sorted, string, &exact_match);
      	struct string_list_item *item;
      
    @@ mailmap.c: void clear_mailmap(struct string_list *map)
     -	if (i < 0) {
     -		/* exact match */
     -		i = -1 - i;
    -+	int exact_match;
    ++	bool exact_match;
     +	int i = string_list_find_insert_index(map, string, &exact_match);
     +	if (exact_match) {
      		if (!string[len])
    @@ refs.c: const char *find_descendant_ref(const char *dirname,
      
     
      ## string-list.c ##
    -@@ string-list.c: int string_list_has_string(const struct string_list *list, const char *string)
    +@@ string-list.c: static size_t get_entry_index(const struct string_list *list, const char *string
    + 		else if (compare > 0)
    + 			left = middle + 1;
    + 		else {
    +-			*exact_match = true;
    ++			if (exact_match)
    ++				*exact_match = true;
    + 			return middle;
    + 		}
    + 	}
    + 
    +-	*exact_match = false;
    ++	if (exact_match)
    ++		*exact_match = false;
    + 	return right;
    + }
    + 
    +@@ string-list.c: 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,
     -				  int negative_existing_index)
    -+				  int *exact_match)
    ++				  bool *exact_match)
      {
    --	int exact_match;
    +-	bool exact_match;
     -	int index = get_entry_index(list, string, &exact_match);
     -	if (exact_match)
     -		index = -1 - (negative_existing_index ? index : 0);
    @@ string-list.c: int string_list_has_string(const struct string_list *list, const
      ## string-list.h ##
     @@ string-list.h: 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);
    + 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,
     -				  int negative_existing_index);
    -+				  int *exact_match);
    ++				  bool *exact_match);
      
      /**
       * Insert a new element to the string_list. The returned pointer can
3:  4d4bdf7cda ! 3:  1cf914fab5 string-list: change "string_list_find_insert_index" return type to "size_t"
    @@ Commit message
         string-list: change "string_list_find_insert_index" return type to "size_t"
     
         As "string_list_find_insert_index" is a simple wrapper of
    -    "get_entry_index", we could simply change its return type to "size_t".
    +    "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-- > 0`.
    +    "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.
    @@ add-interactive.c
     @@ add-interactive.c: static void find_unique_prefixes(struct prefix_item_list *list)
      static ssize_t find_unique(const char *string, struct prefix_item_list *list)
      {
    - 	int exact_match;
    + 	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;
    @@ mailmap.c
     @@ mailmap.c: static struct string_list_item *lookup_prefix(struct string_list *map,
      					      const char *string, size_t len)
      {
    - 	int exact_match;
    + 	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) {
    @@ mailmap.c: static struct string_list_item *lookup_prefix(struct string_list *map
      	 * real location of the key if one exists.
      	 */
     -	while (0 <= --i && i < map->nr) {
    -+	while (i-- > 0 && i < map->nr) {
    ++	while (i-- && i < map->nr) {
      		int cmp = strncasecmp(map->items[i].string, string, len);
      		if (cmp < 0)
      			/*
    @@ refs.c: const char *find_descendant_ref(const char *dirname,
      
     
      ## string-list.c ##
    -@@ string-list.c: int string_list_has_string(const struct string_list *list, const char *string)
    +@@ string-list.c: 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,
    --				  int *exact_match)
    +-				  bool *exact_match)
     +size_t string_list_find_insert_index(const struct string_list *list, const char *string,
    -+				     int *exact_match)
    ++				     bool *exact_match)
      {
      	return get_entry_index(list, string, exact_match);
      }
    @@ string-list.h
     @@ string-list.h: 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);
    + 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,
    --				  int *exact_match);
    +-				  bool *exact_match);
     +size_t string_list_find_insert_index(const struct string_list *list, const char *string,
    -+				     int *exact_match);
    ++				     bool *exact_match);
      
      /**
       * Insert a new element to the string_list. The returned pointer can
4:  9f2b55fb41 = 4:  8a445549dd refs: enable sign compare warnings check
-- 
2.51.0