On Sun, Feb 09, 2025 at 08:12:12AM +0000, Zejun Zhao wrote:
> Some assigned variables are mistyped as `int`, including
>
> - those whose values come from a system function returning `size_t`,
>
> - those that are used for array indexing,
>
> - those that represent length/size/distance,
>
> some of which will trigger -Wsign-comparison warnings.
>
> Change some of them to `size_t`/`unsigned`.
>
> Signed-off-by: Zejun Zhao <jelly.zhao.42@xxxxxxxxx>
> ---
> apply.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/apply.c b/apply.c
> index 831b338155..b4ae74a5fb 100644
> --- a/apply.c
> +++ b/apply.c
> @@ -1087,7 +1087,7 @@ static int gitdiff_index(struct gitdiff_data *state,
> * and optional space with octal mode.
> */
> const char *ptr, *eol;
> - int len;
> + size_t len;
> const unsigned hexsz = the_hash_algo->hexsz;
>
> ptr = strchr(line, '.');
This is storing the result of `ptr - line`, which will be a positive
integer. It's later passed to `memcpy()`, which expects a `size_t`.
> @@ -2320,7 +2320,8 @@ static void update_pre_post_images(struct image *preimage,
> {
> struct image fixed_preimage = IMAGE_INIT;
> size_t insert_pos = 0;
> - int i, ctx, reduced;
> + int i, reduced;
> + size_t ctx;
> const char *fixed;
>
> /*
`ctx` is indexing an array and counts against `preimage->line_nr`, which
is a `size_t`, too.
> @@ -2492,7 +2493,7 @@ static int match_fragment(struct apply_state *state,
> struct strbuf fixed = STRBUF_INIT;
> char *fixed_buf;
> size_t fixed_len;
> - int preimage_limit;
> + size_t preimage_limit;
> int ret;
>
> if (preimage->line_nr + current_lno <= img->line_nr) {
This one stores `struct image::line_nr`, which is a `size_t`.
> @@ -2706,7 +2707,7 @@ static int find_pos(struct apply_state *state,
> {
> int i;
> unsigned long backwards, forwards, current;
> - int backwards_lno, forwards_lno, current_lno;
> + size_t backwards_lno, forwards_lno, current_lno;
>
> /*
> * When running with --allow-overlap, it is possible that a hunk is
These are a bit curious, as they store `line`, which is itself an `int`
parameter. As far as I understand, the only caller is also only ever
passing a positive integer here.
> @@ -2791,7 +2792,7 @@ static int find_pos(struct apply_state *state,
> */
> static void update_image(struct apply_state *state,
> struct image *img,
> - int applied_pos,
> + size_t applied_pos,
> struct image *preimage,
> struct image *postimage)
> {
> @@ -2803,7 +2804,7 @@ static void update_image(struct apply_state *state,
> size_t remove_count, insert_count, applied_at = 0;
> size_t result_alloc;
> char *result;
> - int preimage_limit;
> + size_t preimage_limit;
>
> /*
> * If we are removing blank lines at the end of img,
The caller makes sure that the function only gets called when the
parameter is a positive integer.
> @@ -4288,19 +4289,19 @@ 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;
This one is curious again, as the type of these variables is an `int`.
This should likely be adapted in tandem if they cannot be negative.
Patrick
