Re: [PATCH] add_submodule_odb: initialize alt_odb list earlier

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]