Skip to content

Commit

Permalink
unpack_trees(): protect the handcrafted in-core index from read_cache()
Browse files Browse the repository at this point in the history
unpack_trees() rebuilds the in-core index from scratch by allocating a new
structure and finishing it off by copying the built one to the final
index.

The resulting in-core index is Ok for most use, but read_cache() does not
recognize it as such.  The function is meant to be no-op if you already
have loaded the index, until you call discard_cache().

This change the way read_cache() detects an already initialized in-core
index, by introducing an extra bit, and marks the handcrafted in-core
index as initialized, to avoid this problem.

A better fix in the longer term would be to change the read_cache() API so
that it will always discard and re-read from the on-disk index to avoid
confusion.  But there are higher level API that have relied on the current
semantics, and they and their users all need to get converted, which is
outside the scope of 'maint' track.

An example of such a higher level API is write_cache_as_tree(), which is
used by git-write-tree as well as later Porcelains like git-merge, revert
and cherry-pick.  In the longer term, we should remove read_cache() from
there and add one to cmd_write_tree(); other callers expect that the
in-core index they prepared is what gets written as a tree so no other
change is necessary for this particular codepath.

The original version of this patch marked the index by pointing an
otherwise wasted malloc'ed memory with o->result.alloc, but this version
uses Linus's idea to use a new "initialized" bit, which is conceptually
much cleaner.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Junio C Hamano committed Aug 24, 2008
1 parent 893d340 commit 913e0e9
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 2 deletions.
3 changes: 2 additions & 1 deletion cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,8 @@ struct index_state {
struct cache_tree *cache_tree;
time_t timestamp;
void *alloc;
unsigned name_hash_initialized : 1;
unsigned name_hash_initialized : 1,
initialized : 1;
struct hash_table name_hash;
};

Expand Down
4 changes: 3 additions & 1 deletion read-cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -1155,7 +1155,7 @@ int read_index_from(struct index_state *istate, const char *path)
size_t mmap_size;

errno = EBUSY;
if (istate->alloc)
if (istate->initialized)
return istate->cache_nr;

errno = ENOENT;
Expand Down Expand Up @@ -1195,6 +1195,7 @@ int read_index_from(struct index_state *istate, const char *path)
* index size
*/
istate->alloc = xmalloc(estimate_cache_size(mmap_size, istate->cache_nr));
istate->initialized = 1;

src_offset = sizeof(*hdr);
dst_offset = 0;
Expand Down Expand Up @@ -1247,6 +1248,7 @@ int discard_index(struct index_state *istate)
cache_tree_free(&(istate->cache_tree));
free(istate->alloc);
istate->alloc = NULL;
istate->initialized = 0;

/* no need to throw away allocated active_cache */
return 0;
Expand Down
1 change: 1 addition & 0 deletions unpack-trees.c
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
state.refresh_cache = 1;

memset(&o->result, 0, sizeof(o->result));
o->result.initialized = 1;
if (o->src_index)
o->result.timestamp = o->src_index->timestamp;
o->merge_size = len;
Expand Down

0 comments on commit 913e0e9

Please sign in to comment.