From: "Shawn O. Pearce" <spearce@spearce.org>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Bjjjrn Steinbrink <B.Steinbrink@gmx.de>,
Junio C Hamano <gitster@pobox.com>, Nicolas Pitre <nico@cam.org>,
git@vger.kernel.org
Subject: Re: [PATCH] index-pack: correctly initialize appended objects
Date: Fri, 25 Jul 2008 11:42:02 -0500 [thread overview]
Message-ID: <20080725164202.GC21117@spearce.org> (raw)
In-Reply-To: <alpine.DEB.1.00.0807251513240.11976@eeepc-johanness>
Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Fri, 25 Jul 2008, Björn Steinbrink wrote:
> > On 2008.07.24 22:21:14 -0700, Junio C Hamano wrote:
> > > Reading get_data_from_pack(), it does rely on hdr_size, idx.offset and
> > > idx.offset of the next entry to be set correctly. The function does
> > > not seem to use type (which the patch is also setting) nor real_type
> > > (which the patch does not set).
> >
> > type is used in get_base_data().
> >
> > > 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?
> >
> > I had thought that resolve_delta() would set that, but it seems that we
> > never call that function like that. Hm...
>
> So, let's add the comment as Nico suggested, and set real_type, too? (And
> it would be smashing if you could verify that the type is indeed correctly
> set to non-delta...)
>
> I think that setting real_type is necessary to have less surprises when
> the code is extended in the future.
The patch looks correct, but it should set real_type too because
I'm pretty sure we use that when we unpack the delta base again if
it was pruned out of memory.
--
Shawn.
next prev parent reply other threads:[~2008-07-25 16:43 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-07-24 17:32 [PATCH] index-pack: correctly initialize appended objects Johannes Schindelin
2008-07-24 18:07 ` Björn Steinbrink
2008-07-25 5:21 ` Junio C Hamano
2008-07-25 10:24 ` Johannes Schindelin
2008-07-25 11:54 ` Nicolas Pitre
2008-07-25 12:01 ` Björn Steinbrink
2008-07-25 12:24 ` Nicolas Pitre
2008-07-25 18:15 ` Junio C Hamano
2008-07-25 11:55 ` Björn Steinbrink
2008-07-25 13:15 ` Johannes Schindelin
2008-07-25 16:42 ` Shawn O. Pearce [this message]
2008-07-25 17:13 ` Björn Steinbrink
2008-07-25 17:20 ` Shawn O. Pearce
2008-07-26 3:04 ` Johannes Schindelin
2008-07-25 11:48 ` Nicolas Pitre
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20080725164202.GC21117@spearce.org \
--to=spearce@spearce.org \
--cc=B.Steinbrink@gmx.de \
--cc=Johannes.Schindelin@gmx.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=nico@cam.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.