All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Miklos Vajna <vmiklos@frugalware.org>
Cc: Paolo Bonzini <bonzini@gnu.org>, Jeff King <peff@peff.net>,
	git@vger.kernel.org
Subject: [PATCH 2/2] unpack_trees(): protect the handcrafted in-core index from read_cache()
Date: Sat, 23 Aug 2008 12:57:30 -0700	[thread overview]
Message-ID: <7vbpzjy05x.fsf_-_@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <7vk5e7y0a0.fsf@gitster.siamese.dyndns.org> (Junio C. Hamano's message of "Sat, 23 Aug 2008 12:55:03 -0700")

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>
---
 cache.h                  |    3 ++-
 read-cache.c             |    4 +++-
 t/t7605-merge-resolve.sh |    2 +-
 unpack-trees.c           |    1 +
 4 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index 2475de9..884fae8 100644
--- a/cache.h
+++ b/cache.h
@@ -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;
 };
 
diff --git a/read-cache.c b/read-cache.c
index 2c03ec3..35fec46 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -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;
@@ -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;
@@ -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;
diff --git a/t/t7605-merge-resolve.sh b/t/t7605-merge-resolve.sh
index 5c53608..f1f86dd 100755
--- a/t/t7605-merge-resolve.sh
+++ b/t/t7605-merge-resolve.sh
@@ -27,7 +27,7 @@ test_expect_success 'setup' '
 	git tag c3
 '
 
-test_expect_failure 'merge c1 to c2' '
+test_expect_success 'merge c1 to c2' '
 	git reset --hard c1 &&
 	git merge -s resolve c2 &&
 	test "$(git rev-parse c1)" != "$(git rev-parse HEAD)" &&
diff --git a/unpack-trees.c b/unpack-trees.c
index cba0aca..ef21c62 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -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;
-- 
1.6.0.51.g078ae

  parent reply	other threads:[~2008-08-23 19:58 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-22  6:36 [bug] git `next' does not do trivial merges Paolo Bonzini
2008-08-22 19:31 ` Jeff King
2008-08-23  6:08   ` Miklos Vajna
2008-08-23  8:14     ` [PATCH] Fix in-index merge Miklos Vajna
2008-08-23  8:17       ` [PATCH] builtin-merge: fail properly when we are in the middle of a conflicted merge Miklos Vajna
2008-08-23  9:01         ` Junio C Hamano
2008-08-23 10:57           ` Miklos Vajna
2008-08-23 19:55             ` Junio C Hamano
2008-08-23 19:56               ` [PATCH 1/2] merge: fix numerus bugs around "trivial merge" area Junio C Hamano
2008-08-24  1:58                 ` Junio C Hamano
2008-08-28 13:43                   ` [PATCH] builtin-merge: avoid run_command_v_opt() for recursive and subtree Miklos Vajna
2008-08-23 19:57               ` Junio C Hamano [this message]
2008-08-23  9:50         ` [PATCH] builtin-merge: fail properly when we are in the middle of a conflicted merge Junio C Hamano
2008-08-23  8:50       ` [PATCH] Fix in-index merge Junio C Hamano
2008-08-23 10:41         ` Miklos Vajna
2008-08-23  9:19       ` Paolo Bonzini
2008-08-23  9:55         ` Junio C Hamano
2008-08-23 10:00           ` Junio C Hamano
2008-08-23 10:41             ` [RFH] two and half potential fixlets to the in-core index handling Junio C Hamano
2008-08-23 18:13               ` Linus Torvalds
2008-08-23 19:14                 ` Junio C Hamano
2008-08-23 19:21                   ` Junio C Hamano

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=7vbpzjy05x.fsf_-_@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=bonzini@gnu.org \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=vmiklos@frugalware.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.