Re: Infinite loop + memory leak in annotate_refs_with_symref_info

[Taylor Blau <me@ttaylorr.com>]

On Tue, Apr 11, 2023 at 04:25:13PM -0400, Taylor Blau wrote:
> > I believe the bug in Git is in connect.c, function
> > parse_feature_value, in the updating of `*offset`: It doesn’t seem to
> > take into account that `feature_list` has already been offset by
> > `*offset`. I believe the update needs to use `*offset +=` instead of
> > `*offset =`. When I make this change, the infinite loop seems to go
> > away, and cloning via Klaus/Dulwich will fail with “invalid index-pack
> > output”. Cloning from github.com works, although I’m not sure if
> > that’s a relevant smoke test in this case.
>
> I'm not sure I understand. Looking at the relevant bits in
> connect.c::parse_feature_value(), it all seems correct to me, since the
> beginning of `feature_list` is adjusted by the current value of
> `*offset`.

Oops. This was exactly[1] the problem as you suggested, I was just
thinking about it backwards. When we write into `*offset`, we need to
take into account that `feature_list` has already been moved forward by
`*offset`.

Obviously the discussion can continue below [1], but just wanted to
correct my wrong here and acknowledge that you were absolutely right in
your original report.

Thanks,
Taylor

[1]: https://lore.kernel.org/git/20230411215845.GA678138@coredump.intra.peff.net/