On Wed, Oct 28, 2015 at 08:24:17AM -0700, Junio C Hamano wrote:
> > Note that we can remove the prepare_alt_odb call from the
> > end. It is guaranteed to be a noop, since we will have
> > called it earlier.
>
> Thanks for a quick and detailed diagnosis and a fix.
>
> The removal is correct, but even without this fix, the order of
> calls in the original should have screamed "bug" loudly at us, I
> think. We shouldn't be reading data from alternates file without
> first preparing the place we read data into.
Yeah, I agree. I spent a long time trying to figure out if that
prepare_alt_odb was actually doing something useful (like if it was
needed to somehow "cement" the new alt into place).
But I don't think it was.
In the majority of cases, it was a noop (we had already prepared when we
looked up the first object). But for other cases...
- if read_info_alternates actually did something, we segfaulted (i.e.,
this bug)
- otherwise, we would prepare on _top_ of what we just added to the
list, which was probably buggy (I didn't dig far enough to see if
prepare_alt_odb() would overwrite what we just added to the list).
So some pretty dark corners of the code. :)
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html