Re: [PATCH] Fix random fast-import errors when compiled with NO_MMAP

[Junio C Hamano <gitster@pobox.com>]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> Btw, even with Shawn's patch, I wonder if the index_data usage is correct.

Hmph.

gfi uses data in a "pack" in two quite different ways.

A new object is written to an unfinalized pack.  Such a pack
already has "struct packed_git" allocated for it and a pointer
to it is held in pack_data.  As far as the core part of git
(that is, sha1_file.c) is concerned, however, this pack does not
even exist.  It is still not part of packed_git list in
sha1_file.c, and read_sha1_file() will not see objects in it, as
no idx into the packfile exists yet.  gfi has a table of objects
in this pack and uses gfi_unpack_entry() function to retrieve
data from it.

A packfile is finalized in end_packfile().  The pack header and
footer is recomputed, an idx file is written, and the pack is
finally registered.  Before that time p->index_data is not even
used for that pack (it is initialized with NULL).

So I do not think "index_data usage" can be incorrect, as there
won't be any index_data usage with unfinalized pack, and the
core part of git would not even have any mmap(2) (nor open fd)
into its idx file before it is finalized.

By the way, I was quite puzzled how the gfi_unpack_entry()
function manages to work correctly when it has to read an object
it deltified based on another object it wrote into the same
unfinalized pack earlier.  It knows where in the unfinalized
pack it wrote the object, so it can find from its own "struct
object_entry" the offset for the object, and calls
unpack_entry() defined in the core to do the rest.

However, most of the core does not really know about the other
objects in this half-built pack.  If the object is a delta,
unpack_delta_entry() needs to find the delta base.  And it needs
to do that without having the idx.

The trick (the code really needs a bit more documentation) is
that gfi never writes anything but OFS_DELTA.  So the core, even
though it does not have the corresponding idx file, does not
have to look up the object (in fact it does not even know what
object to look up for the base, it only knows the offset).