From: Zejun Zhao <jelly.zhao.42@gmail.com>
To: jelly.zhao.42@gmail.com
Cc: git@vger.kernel.org, gitster@pobox.com, newren@gmail.com,
ps@pks.im, karthik.188@gmail.com
Subject: [GSOC][PATCH v3 0/6] apply: address -Wsign-comparison warnings
Date: Sun, 16 Feb 2025 07:28:37 +0000 [thread overview]
Message-ID: <20250216072843.72385-1-jelly.zhao.42@gmail.com> (raw)
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
next prev parent reply other threads:[~2025-02-16 7:29 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-05 1:40 [GSOC][PATCH] apply: address -Wsign-comparison warnings Zejun Zhao
2025-02-05 7:47 ` Patrick Steinhardt
2025-02-05 15:42 ` Zejun Zhao
2025-02-05 12:58 ` Junio C Hamano
2025-02-05 16:49 ` Zejun Zhao
2025-02-09 8:12 ` [GSOC][PATCH v2 0/6] " Zejun Zhao
2025-02-09 8:12 ` [GSOC][PATCH v2 1/6] apply: change fields in `apply_state` to unsigned Zejun Zhao
2025-02-13 9:51 ` Karthik Nayak
2025-02-13 18:39 ` Junio C Hamano
2025-02-09 8:12 ` [GSOC][PATCH v2 2/6] apply: change some variables from `int` to `size_t` Zejun Zhao
2025-02-13 6:08 ` Patrick Steinhardt
2025-02-16 7:12 ` [GSOC][PATCH] apply: address -Wsign-comparison warnings Zejun Zhao
2025-02-18 17:49 ` Junio C Hamano
2025-02-21 6:37 ` Zejun Zhao
2025-02-21 17:11 ` Junio C Hamano
2025-02-23 17:36 ` Zejun Zhao
2025-02-24 14:27 ` Junio C Hamano
2025-02-25 3:24 ` Zejun Zhao
2025-02-25 12:53 ` Junio C Hamano
2025-02-09 8:12 ` [GSOC][PATCH v2 3/6] apply: do a typecast to eliminate warnings Zejun Zhao
2025-02-09 8:12 ` [GSOC][PATCH v2 4/6] apply: cast some ptrdiff_t's to size_t's Zejun Zhao
2025-02-09 8:12 ` [GSOC][PATCH v2 5/6] apply: use `size_t` loop counters Zejun Zhao
2025-02-13 6:08 ` Patrick Steinhardt
2025-02-09 8:12 ` [GSOC][PATCH v2 6/6] apply: enable -Wsign-comparison checks Zejun Zhao
2025-02-16 7:28 ` Zejun Zhao [this message]
2025-02-16 7:28 ` [PATCH v3 1/6] apply: correct type misuse in `apply.h` Zejun Zhao
2025-02-16 7:28 ` [PATCH v3 2/6] apply: change some variables from `int` to `size_t` Zejun Zhao
2025-02-16 7:28 ` [PATCH v3 3/6] apply: do a typecast to eliminate warnings Zejun Zhao
2025-02-16 7:28 ` [PATCH v3 4/6] apply: cast some ptrdiff_t's to size_t's Zejun Zhao
2025-02-16 7:28 ` [PATCH v3 5/6] apply: use `size_t` loop counters Zejun Zhao
2025-02-16 7:28 ` [PATCH v3 6/6] apply: enable -Wsign-comparison checks Zejun Zhao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250216072843.72385-1-jelly.zhao.42@gmail.com \
--to=jelly.zhao.42@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=karthik.188@gmail.com \
--cc=newren@gmail.com \
--cc=ps@pks.im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).