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@xxxxxxxxxxxxxxxxxxxxxxx/
