Re: [PATCH] xdiff: avoid arithmetic overflow in xdl_get_hunk()

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

René Scharfe <l.s.r@web.de> writes:

> Am 14.03.25 um 23:28 schrieb Junio C Hamano:
>> René Scharfe <l.s.r@web.de> writes:
>>
>>>  t/t4055-diff-context.sh | 10 ++++++++++
>>>  xdiff/xemit.c           |  8 +++++++-
>>>  2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> Oh, I love a patch like this that is well thought out to carefully
>> check the bounds, instead of blindly say "ah, counting number of
>> things in size_t solves everything" ;-)
>
> Converting xdiff from long to size_t is still a good idea, I think, ...

Oh, no question about that, especially if the number of counted
things are somehow proportionally related to the size of in-core
memory regions in any way.

What I do *not* like is the recent trend in the patches I see.  They
stop thinking there once they blindly replace int or unsigned or
whatever with size_t and the compiler stops warning.  Even though
compiler warnings can be a useful tool when there is very little
false positives, they are merely tools to improve the code, but I
see more and more confused patches that seem to think squelching
warnings is the goal in itself, without thinking if the resulting
code is actually improved.

And I didn't see that in this patch.  The patch was actually written
with real goal of improving the code in mind.

> but
> would be lot more effort and thus more risky.  

Perhaps, perhaps not.

> Comparisons to upstream
> would become a lot more noisy as well.

I am not sure how much of that matters these days, though.  Are they
still active, or is the code perfect and pretty much done?  I somehow
had the impression it has been the latter for a long time...

Thanks.