Re: [GSOC][PATCH] apply: address -Wsign-comparison warnings

[Junio C Hamano <gitster@pobox.com>]

Zejun Zhao <jelly.zhao.42@gmail.com> writes:

> On Thu, Feb 13 2025 07:08:13 +0100, Patrick Steinhardt wrote,
>> > @@ -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.
>
> They do store an `int` parameter, which is `line`. However, we cannot change 
> `line` to unsigned since it can temporarily store an negative value before 
> the assignments to these `*_lno` happen.

Correct.  Doesn't it mean that a change that makes these line-number
variables to size_t is wrong?  Of course the change is not made to
break the code but may be to please some code paths in other parts
of the system that wants these line-number variables that are
currently "int" to compare or assign without range-checking with
"size_t" quantity or variable, but then shouldn't we fix these
places, not the types of these line-number variables that use the
platform natural integers?

THanks.