Skip to content

Commit 913e0e9

Browse files
committed
unpack_trees(): protect the handcrafted in-core index from read_cache()
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 <[email protected]>
1 parent 893d340 commit 913e0e9

File tree

3 files changed

+6
-2
lines changed

3 files changed

+6
-2
lines changed

cache.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,8 @@ struct index_state {
222222
struct cache_tree *cache_tree;
223223
time_t timestamp;
224224
void *alloc;
225-
unsigned name_hash_initialized : 1;
225+
unsigned name_hash_initialized : 1,
226+
initialized : 1;
226227
struct hash_table name_hash;
227228
};
228229

read-cache.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1155,7 +1155,7 @@ int read_index_from(struct index_state *istate, const char *path)
11551155
size_t mmap_size;
11561156

11571157
errno = EBUSY;
1158-
if (istate->alloc)
1158+
if (istate->initialized)
11591159
return istate->cache_nr;
11601160

11611161
errno = ENOENT;
@@ -1195,6 +1195,7 @@ int read_index_from(struct index_state *istate, const char *path)
11951195
* index size
11961196
*/
11971197
istate->alloc = xmalloc(estimate_cache_size(mmap_size, istate->cache_nr));
1198+
istate->initialized = 1;
11981199

11991200
src_offset = sizeof(*hdr);
12001201
dst_offset = 0;
@@ -1247,6 +1248,7 @@ int discard_index(struct index_state *istate)
12471248
cache_tree_free(&(istate->cache_tree));
12481249
free(istate->alloc);
12491250
istate->alloc = NULL;
1251+
istate->initialized = 0;
12501252

12511253
/* no need to throw away allocated active_cache */
12521254
return 0;

unpack-trees.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
376376
state.refresh_cache = 1;
377377

378378
memset(&o->result, 0, sizeof(o->result));
379+
o->result.initialized = 1;
379380
if (o->src_index)
380381
o->result.timestamp = o->src_index->timestamp;
381382
o->merge_size = len;

0 commit comments

Comments
 (0)