git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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


  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).