[GSOC][PATCH v3 0/6] apply: address -Wsign-comparison warnings
- From
- Zejun Zhao <jelly.zhao.42@gmail.com>
- Date
- Feb 16, 2025, 07:28 UTC
- Message-ID
- <20250216072843.72385-1-jelly.zhao.42@gmail.com>
- In-Reply-To
- <20250205014055.737190-1-jelly.zhao.42@gmail.com>
This is the v3 of this patch set.
Changes from v2:
- change `patch::lines_added/deleted` to unsigned
- use `unsigned int` instead of `unsigned`
- separate irrelevant changes from the "loop counter" commit
Below are the cover letter from the v2.
There are several -Wsign-comparison warnings in "apply.c", which can be classified into the following three types:
1. comparing a length of `size_t` type with a `ptrdiff_t` value
2. comparing a length of `size_t` type with a length of `int` type
3. comparing a loop counter `i` of `int` type with an unsigned loop bound
Fix these warnings following one basic principle: do not touch the relevant logics and keep the behaviors of the code. Adopt three different strategies for each of the above three types:
1. cast the `ptrdiff_t` values to `size_t` type
2. try to change the type of the `int` length to `size_t` (may fall back to Strategy 1 if the variable is not guaranteed to be unsigned)
3. use a loop counter `i` of `size_t` type
Zejun Zhao (6): apply: correct type misuse in `apply.h` apply: change some variables from `int` to `size_t` apply: do a typecast to eliminate warnings apply: cast some ptrdiff_t's to size_t's apply: use `size_t` loop counters apply: enable -Wsign-comparison checks
apply.c | 69 +++++++++++++++++++++++++++------------------------------ apply.h | 6 ++--- 2 files changed, 36 insertions(+), 39 deletions(-)
Range-diff against v2:
1: 16ee2f93a3 ! 1: a5985e0e16 apply: change fields in `apply_state` to unsigned
@@ Metadata
Author: Zejun Zhao <jelly.zhao.42@gmail.com>
## Commit message ##
- apply: change fields in `apply_state` to unsigned
+ apply: correct type misuse in `apply.h`
- `.max_change` and `.max_len` of `apply_state` are only used as unsigned
- integers. Misuse of `int` type would cause -Wsign-comparison warnings.
+ There are a few `int` fields of structs in `apply.h` that are only used
+ as unsigned integer and are only assigned unsigned integer value. Some
+ of these misuse of `int` type would cause -Wsign-comparison warnings.
Fix this by
- - change `.max_change`'s type to `unsigned` since it's just a counter
+ - change `apply_state::max_change`'s type to `unsigned int` since it's
+ just a counter of lines
- - change `.max_len`'s type to `size_t` since it's a length
+ - change `apply_state::max_len`'s type to `size_t` since it's storing
+ a length
- - change the types of relevant variables in function `show_stats`
+ - change `patch::lines_added/deleted`'s types to `unsigned int` since
+ they are just counters of lines
+
+ - change the types of relevant variables in function `show_stats` and
+ `patch_stats`
Note that `printf`'s format string requires us to do some typecast
(from `size_t` to `int`) on `max` in function `show_stats`. This is
@@ apply.c: static void show_stats(struct apply_state *state, struct patch *patch)
char *cp = patch->new_name ? patch->new_name : patch->old_name;
- int max, add, del;
+ size_t max;
-+ unsigned add, del;
++ unsigned int add, del;
quote_c_style(cp, &qname, NULL, 0);
@@ apply.c: static void show_stats(struct apply_state *state, struct patch *patch)
if (state->max_change > 0) {
- int total = ((add + del) * max + state->max_change / 2) / state->max_change;
-+ unsigned total = ((add + del) * max + state->max_change / 2) / state->max_change;
++ unsigned int total = ((add + del) * max + state->max_change / 2) / state->max_change;
add = (add * max + state->max_change / 2) / state->max_change;
del = total - add;
}
+@@ apply.c: static void summary_patch_list(struct patch *patch)
+
+ static void patch_stats(struct apply_state *state, struct patch *patch)
+ {
+- int lines = patch->lines_added + patch->lines_deleted;
++ unsigned int lines = patch->lines_added + patch->lines_deleted;
+
+ if (lines > state->max_change)
+ state->max_change = lines;
## apply.h ##
@@ apply.h: struct apply_state {
@@ apply.h: struct apply_state {
*/
- int max_change;
- int max_len;
-+ unsigned max_change;
++ unsigned int max_change;
+ size_t max_len;
/*
* Records filenames that have been touched, in order to handle
+@@ apply.h: struct patch {
+ int is_new, is_delete; /* -1 = unknown, 0 = false, 1 = true */
+ int rejected;
+ unsigned ws_rule;
+- int lines_added, lines_deleted;
++ unsigned int lines_added, lines_deleted;
+ int score;
+ int extension_linenr; /* first line specifying delete/new/rename/copy */
+ unsigned int is_toplevel_relative:1;
2: 962ab421cf ! 2: b73f3e9fae apply: change some variables from `int` to `size_t`
@@ Commit message
some of which will trigger -Wsign-comparison warnings.
- Change some of them to `size_t`/`unsigned`.
+ Change some of them to `size_t`/`unsigned int`.
Signed-off-by: Zejun Zhao <jelly.zhao.42@gmail.com>
@@ apply.c: static void update_pre_post_images(struct image *preimage,
const char *fixed;
/*
+@@ apply.c: static int line_by_line_fuzzy_match(struct image *img,
+ struct image *postimage,
+ unsigned long current,
+ int current_lno,
+- int preimage_limit)
++ size_t preimage_limit)
+ {
+ int i;
+ size_t imgoff = 0;
@@ apply.c: static int match_fragment(struct apply_state *state,
struct strbuf fixed = STRBUF_INIT;
char *fixed_buf;
@@ apply.c: static void update_image(struct apply_state *state,
/*
* If we are removing blank lines at the end of img,
-@@ apply.c: static void summary_patch_list(struct patch *patch)
-
- static void patch_stats(struct apply_state *state, struct patch *patch)
- {
-- int lines = patch->lines_added + patch->lines_deleted;
-+ unsigned lines = patch->lines_added + patch->lines_deleted;
-
+@@ apply.c: static void patch_stats(struct apply_state *state, struct patch *patch)
if (lines > state->max_change)
state->max_change = lines;
if (patch->old_name) {
3: 09f42cbd5a = 3: 85938a18b1 apply: do a typecast to eliminate warnings
4: 1142213871 = 4: ba31e9406d apply: cast some ptrdiff_t's to size_t's
5: 8e27fb9c08 ! 5: 351e561843 apply: use `size_t` loop counters
@@ apply.c: static void update_pre_post_images(struct image *preimage,
if (!(postimage->line[i].flag & LINE_COMMON)) {
@@ apply.c: static int line_by_line_fuzzy_match(struct image *img,
- struct image *postimage,
- unsigned long current,
int current_lno,
-- int preimage_limit)
-+ size_t preimage_limit)
+ size_t preimage_limit)
{
- int i;
+ size_t i;
6: 60155b1aeb = 6: cdb3e806a1 apply: enable -Wsign-comparison checks-- 2.43.0