Re: Delitifier broken (Re: diff-core segfault)

From: Nicolas Pitre <nico@cam.org>
Date: Dec 12, 2005, 21:28 UTC
Message-ID: <Pine.LNX.4.64.0512121620380.26663@localhost.localdomain>
In-Reply-To: <7virtui1kj.fsf_-_@assigned-by-dhcp.cox.net>

On Mon, 12 Dec 2005, Junio C Hamano wrote:

Show 13 quoted lines

> Junio C Hamano <junkio@cox.net> writes:
> 
> > Darrin Thompson <darrint@progeny.com> writes:
> >
> >> Could someone confirm that this exists on more recent git heads and fix
> >> if needed?
> >
> > (1) Yup.  I can reproduce it.
> > (2) Will look into it when able.
> 
> This is not just "diff".  Our deltify code is half-broken, and
> in the worst case this can corrupt our packs if an empty blob is
> involved.

I would say involving an empty blob with deltas _is_ the bug in the first place. Please don't let that happen.

Especially with pack files, an empty blob can be represented with a _single_ byte. A delta must always be against something else and simply storing the reference for the object the delta is against will always use at least 20 bytes even for empty ones.

Show 10 quoted lines

> The problem is if from_size or to_size is empty, it does not
> produce any.
> 
>         if (!from_size || !to_size || delta_prepare(from_buf, from_size, &bdf))
>                 return NULL;
> 	
> 
> I think either we need to make the users more careful or fix
> deltifier to produce trivial delta.  I'd vote for the latter;
> let me rig up something.

If my opinion is still of any weight I'd strongly vote for the former. A delta against an empty object, or a delta that produces an empty object simply makes no sense since it is always suboptimal compared to storing the non deltified object (or finding another object to deltify against). Allowing empty deltas only paper over another more fundamental bug IMHO.

Nicolas