Re: [PATCH] index-pack: correctly initialize appended objects

[Nicolas Pitre <nico@cam.org>]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1618 bytes --]

On Fri, 25 Jul 2008, Björn Steinbrink wrote:

> On 2008.07.25 07:54:49 -0400, Nicolas Pitre wrote:
> > On Fri, 25 Jul 2008, Johannes Schindelin wrote:
> > 
> > > Hi,
> > > 
> > > On Thu, 24 Jul 2008, Junio C Hamano wrote:
> > > 
> > > > The function does not seem to use type (which the patch is also setting) 
> > > > nor real_type (which the patch does not set).
> > > > 
> > > > However, the code checks objects[nth].real_type all over the place in 
> > > > the code.  Doesn't the lack of real_type assignment in 
> > > > append_obj_to_pack() affect them in any way?
> > > 
> > > >From staring at the code, I thought that real_type was set in 
> > > resolve_delta(), but I may be wrong.
> > > 
> > > The safer thing would be to set it, but I am not quite sure if we can use 
> > > "type" directly, or if type can be "delta" for an object that is used to 
> > > complete the pack, and therefore stored as a non-delta.
> > 
> > Objects to complete the pack are always non delta, so the type and 
> > real_type should be the same.  However that shouldn't matter since at 
> > that point the object array is not walked anymore, at least not for 
> > appended objects, and therefore initializing the type at that point is 
> > redundant.
> 
> Is that still true when the object has been pruned due to memory
> constraints set by deltaBaseCacheLimit? AFAICT when reloading the data
> for the object, we end up in get_base_data, which at least checks
> obj->type.

yeah, true.  I don't really have this new code path in my head yet.

In any case, appended objects should have type = real_type = non delta 
type.


Nicolas