git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[GSOC][PATCH v3 0/6] apply: address -Wsign-comparison warnings

From
ZZZejun 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
Previous: Zejun ZhaoNext: Zejun Zhao
Message 25 of 31 in “apply: address -Wsign-comparison warnings”
  1. Zejun ZhaoFeb 5, 2025
  2. Patrick SteinhardtFeb 5, 2025
  3. Zejun ZhaoFeb 5, 2025
  4. Junio C HamanoFeb 5, 2025
  5. Zejun ZhaoFeb 5, 2025
  6. [GSOC][PATCH v2 0/6] apply: address -Wsign-comparison warningsZejun Zhao, Feb 9, 2025
  7. [GSOC][PATCH v2 1/6] apply: change fields in `apply_state` to unsignedZejun Zhao, Feb 9, 2025
  8. Karthik NayakFeb 13, 2025
  9. Junio C HamanoFeb 13, 2025
  10. [GSOC][PATCH v2 2/6] apply: change some variables from `int` to `size_t`Zejun Zhao, Feb 9, 2025
  11. Patrick SteinhardtFeb 13, 2025
  12. Zejun ZhaoFeb 16, 2025
  13. Junio C HamanoFeb 18, 2025
  14. Zejun ZhaoFeb 21, 2025
  15. Junio C HamanoFeb 21, 2025
  16. Zejun ZhaoFeb 23, 2025
  17. Junio C HamanoFeb 24, 2025
  18. Zejun ZhaoFeb 25, 2025
  19. Junio C HamanoFeb 25, 2025
  20. [GSOC][PATCH v2 3/6] apply: do a typecast to eliminate warningsZejun Zhao, Feb 9, 2025
  21. [GSOC][PATCH v2 4/6] apply: cast some ptrdiff_t's to size_t'sZejun Zhao, Feb 9, 2025
  22. [GSOC][PATCH v2 5/6] apply: use `size_t` loop countersZejun Zhao, Feb 9, 2025
  23. Patrick SteinhardtFeb 13, 2025
  24. [GSOC][PATCH v2 6/6] apply: enable -Wsign-comparison checksZejun Zhao, Feb 9, 2025
  25. [GSOC][PATCH v3 0/6] apply: address -Wsign-comparison warningsZejun Zhao, Feb 16, 2025
  26. 1/6 apply: correct type misuse in `apply.h`Zejun Zhao, Feb 16, 2025
  27. 2/6 apply: change some variables from `int` to `size_t`Zejun Zhao, Feb 16, 2025
  28. 3/6 apply: do a typecast to eliminate warningsZejun Zhao, Feb 16, 2025
  29. 4/6 apply: cast some ptrdiff_t's to size_t'sZejun Zhao, Feb 16, 2025
  30. 5/6 apply: use `size_t` loop countersZejun Zhao, Feb 16, 2025
  31. 6/6 apply: enable -Wsign-comparison checksZejun Zhao, Feb 16, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.