[PATCH] checkout: do not write bogus reflog entry out

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 6, 2011, 23:17 UTC
Message-ID: <7vk4bvhuco.fsf@alter.siamese.dyndns.org>

The code called resolve_ref("HEAD") and stored the result in old.path to record which branch it used to be on, so that it can say between which branches the switch took place in the reflog.

However, lookup_commit_reference_gently() is called immediately after old.path is initialized. This call can prime replace-object machinery which in turn collects the loose refs, and calls resolve_ref() inside. As the string returned from resolve_ref() is a static buffer that is local to it, this ended up clobbering the value of old.path. The reflog either recorded a wrong "switched from" branch (when the last call to resolve_ref happens to be from the branch namespace), or recorded from a detached HEAD (otherwise). Neither is correct.

Store the returned string away with xstrdup() to avoid this.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This was present ever since checkout was rewritten in C at 782c2d6
   (Build in checkout, 2008-02-07), but probably got a lot easier to
   trigger recently due to the addition of object replacement code.

   As it depends on the order readdir() in refs.c::get_ref_dir() returns
   the entries, there is no universally reproducible test, but I found it
   in real life inside my git.git primary working repository.

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 28cdc51..48de146 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -715,10 +715,12 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 	unsigned char rev[20];
 	int flag;
 	memset(&old, 0, sizeof(old));
-	old.path = resolve_ref("HEAD", rev, 0, &flag);
+	old.path = xstrdup(resolve_ref("HEAD", rev, 0, &flag));
 	old.commit = lookup_commit_reference_gently(rev, 1);
-	if (!(flag & REF_ISSYMREF))
+	if (!(flag & REF_ISSYMREF)) {
+		free((char *)old.path);
 		old.path = NULL;
+	}
 
 	if (old.path && !prefixcmp(old.path, "refs/heads/"))
 		old.name = old.path + strlen("refs/heads/");
@@ -741,6 +743,7 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 	update_refs_for_switch(opts, &old, new);
 
 	ret = post_checkout_hook(old.commit, new->commit, 1);
+	free((char *)old.path);
 	return ret || opts->writeout_error;
 }