Skip to content

enhance string-list API to fix sign compare warnings

shejialuo git 3 versions – In review

Get series with b4
b4 am aONhmrE0otiyZ16f@ArchLinux

Versions newest first · linked by author and title

  1. v3 Latest4 patches 5 repliesIn review
  2. v24 patches 15 repliesSuperseded
  3. v14 patches 8 repliesSuperseded

Changes in v3 from the cover letter

1. Enhance [PATCH v2 2/4] commit message to express the motivation is avoid overflow.
2. Add comments for string_list_find_insert_index function.

Patches in v3 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/4string-list: use bool instead of int for "exact_match"string-list.c string-list.h+11 -100 replies
2/4string-list: replace negative index encoding with "exact_match" parameteradd-interactive.c mailmap.c refs.c +2 more+15 -170 replies
3/4string-list: change "string_list_find_insert_index" return type to "size_t"add-interactive.c mailmap.c refs.c +2 more+14 -111 reply
4/4refs: enable sign compare warnings checkrefs.c+3 -60 replies

Reviews trailers per patch

None of the 4 patches has a Reviewed-by or Acked-by of their own; the series has one for all of them.

PatchCollin Funk
SeriesR-b
1/4
2/4
3/4
4/4

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v3 thread

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

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 v2:

1. Enhance [PATCH v2 2/4] commit message to express the motivation is avoid overflow.
2. Add comments for string_list_find_insert_index function.

---

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     | 12 +++++++++---
 5 files changed, 35 insertions(+), 36 deletions(-)

Range-diff against v2:

1:  c3786fa386 = 1:  bdcac9b5fa string-list: use bool instead of int for "exact_match"
2:  7ac8fd69c0 ! 2:  0d6d09c8b0 string-list: replace negative index encoding with "exact_match" parameter

@@ Metadata
## Commit message ##
string-list: replace negative index encoding with "exact_match" parameter

- 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. This approach has several limitations:
+ The "string_list_find_insert_index()" function is used to determine
+ the correct insertion index for a new string within the string list.
+ The function also doubles up to convey if the string is already
+ existing in the list, this is done by returning a negative index
+ "-1 -index". Users are expected to decode this information. This
+ approach has several limitations:

- 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. It requires the callers to look into the detail of the function to
+ understand how to decode the negative index encoding.
+ 2. Using int for indices can cause overflow issues when dealing with
+ large string lists.

To address these limitations, change the function to return size_t for
the index value and use a separate bool parameter to indicate whether
3: 1cf914fab5 ! 3: 9bfe17ab19 string-list: change "string_list_find_insert_index" return type to "size_t"
@@ string-list.h: void string_list_remove_empty_items(struct string_list *list, int
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);
++
++/**
++ * Find the index at which a new element should be inserted into the
++ * string_list to maintain sorted order. If exact_match is not NULL,
++ * it will be set to true if the string already exists in the list.
++ */
+size_t string_list_find_insert_index(const struct string_list *list, const char *string,
+ bool *exact_match);

4: 8a445549dd = 4: 2a602954f2 refs: enable sign compare warnings check

Signature · 1 line
2.51.0