On Thu, Aug 11, 2016 at 01:02:52AM -0400, Jeff King wrote:

> > > + *   2. Updating our size; check_object() will have filled in the size 
> > > of our
> > > + *      delta, but a non-delta object needs it true size.
> > 
> > Excellent point.
> I was not clever enough to think of it; the pack-objects code is filled
> with nice assertions (Thanks, Nico!) that help out when you are stupid. :)
> One thing to be careful of is that there are more things this
> drop_reused_delta() should be doing. But I looked through the rest of
> check_object() and could not find anything else.

Argh, I spoke too soon.

It is true that the size lookup is the only part of check_object()
we skip if we are reusing the delta. But what I didn't notice is that
when we choose to reuse a delta, we overwrite entry->type (the real
type!) with the in_pack_type (OFS_DELTA, etc). We need to undo that so
that later stages see the real type.

I'm not sure how my existing tests worked (I confirmed that they do
indeed break the delta). It may be that only some code paths actually
care about the real type. But when playing with the depth-limit (which
uses the same drop_reused_delta() helper), I managed to make some pretty
broken packs.

So please disregard the v4 patch I just sent; I haven't triggered it,
but I imagine it has the same problem, and I just didn't manage to
trigger it.

I'll clean that up and send out a new series.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to