Re: [PATCH 1/3] apply: get rid of useless x < 0 comparison on a size_t type

[Giuseppe Bilotta <giuseppe.bilotta@gmail.com>]

On Sun, Nov 6, 2011 at 7:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:
>>
>> This check was originally added in v1.6.5-rc0~53^2 by Giuseppe Bilotta
>> while adding an option to git-apply to ignore whitespace differences.
>
> I agree that these quantities can never be negative, so I'll apply the
> patch as is.

I agree too. In fact I spotted this some time ago when I started
compiling git with Clang but never found the time to look into it.

> But I have this suspicion that this was a rather sloppy defensive check to
> protect this codepath against potential breakage in another codepath (most
> likely update_pre_post_images() touched by the same patch) that adjusts
> image->line[].len the caller of this function uses to feed these two
> parameters. Giuseppe may have been not confident enough that the code
> added to that function ensures not to undershoot when it reduces "len", or
> something.
>
> Giuseppe, can you explain what is going on?

No, I can't, so I guess the sloppy coding is the right motivation. I
remember working this patch from a rather older submission that was
never followed-up to, so my guess is that I just forgot to clean it up
properly, and during review the focus was obviously on other aspects
of the submission too. Also, having a look at the current caller of
the function, I don't see how the check would even be needed.

-- 
Giuseppe "Oblomov" Bilotta