git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/3] create_symref(): do not assume pathname from git_path() persists long enough

From
Junio C Hamano <junkio@cox.net>
Date
Jan 27, 2007, 01:49 UTC
Message-ID
<7vr6thqlwz.fsf@assigned-by-dhcp.cox.net>
In-Reply-To
<11698503722922-git-send-email-nico@cam.org>

Being lazy to rely on the cycling N buffers mkpath() and friends return is nice in general, but it makes it too easy to introduce new bugs that are "mysterious".

Introduction of read_ref() in create_symref() after calling git_path() to get the git_HEAD value (i.e. the path to create a new symref at) consumed more than the available buffers and broke a later call to mkpath() that derives lockpath from it.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 * t7201-co.sh caught three failures.  The problem this patch is
   fixing overwrote and gave ".lock" in lockpath, then open() on
   that path failed and gave fd = -1 (because I scratched my
   head and re-run the test without cleaning, and O_EXCL
   triggered), which threw write_in_full() into infinite loop,
   which will be fixed with the next patch.
 refs.c |   20 +++++++++++---------
 1 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/refs.c b/refs.c
index 5913185..4a52308 100644
--- a/refs.c
+++ b/refs.c
@@ -992,7 +992,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
 	const char *lockpath;
 	char ref[1000];
 	int fd, len, written;
-	const char *git_HEAD = git_path("%s", ref_target);
+	char *git_HEAD = xstrdup(git_path("%s", ref_target));
 	unsigned char old_sha1[20], new_sha1[20];
 
 	if (logmsg && read_ref(ref_target, old_sha1))
@@ -1010,36 +1010,38 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
 	len = snprintf(ref, sizeof(ref), "ref: %s\n", refs_heads_master);
 	if (sizeof(ref) <= len) {
 		error("refname too long: %s", refs_heads_master);
-		return -1;
+		goto error_free_return;
 	}
 	lockpath = mkpath("%s.lock", git_HEAD);
 	fd = open(lockpath, O_CREAT | O_EXCL | O_WRONLY, 0666);
 	if (fd < 0) {
 		error("Unable to open %s for writing", lockpath);
-		return -5;
+		goto error_free_return;
 	}
 	written = write_in_full(fd, ref, len);
 	close(fd);
 	if (written != len) {
-		unlink(lockpath);
 		error("Unable to write to %s", lockpath);
-		return -2;
+		goto error_unlink_return;
 	}
 	if (rename(lockpath, git_HEAD) < 0) {
-		unlink(lockpath);
 		error("Unable to create %s", git_HEAD);
-		return -3;
+		goto error_unlink_return;
 	}
 	if (adjust_shared_perm(git_HEAD)) {
-		unlink(lockpath);
 		error("Unable to fix permissions on %s", lockpath);
-		return -4;
+	error_unlink_return:
+		unlink(lockpath);
+	error_free_return:
+		free(git_HEAD);
+		return -1;
 	}
 
 	done:
 	if (logmsg && !read_ref(refs_heads_master, new_sha1))
 		log_ref_write(ref_target, old_sha1, new_sha1, logmsg);
 
+	free(git_HEAD);
 	return 0;
 }
 
-- 
1.5.0.rc2.g8ac6-dirty
Previous: Nicolas Pitre
Message 8 of 8 in “make reflog filename independent from struct ref_lock”
  1. 1/7 make reflog filename independent from struct ref_lockNicolas Pitre, Jan 26, 2007
  2. 2/7 lock_ref_sha1_basic(): remember the original name of a ref when resolving itNicolas Pitre, Jan 26, 2007
  3. 3/7 enable separate reflog for HEADNicolas Pitre, Jan 26, 2007
  4. 4/7 add reflog entries for HEAD when detachedNicolas Pitre, Jan 26, 2007
  5. 5/7 move create_symref() past log_ref_write()Nicolas Pitre, Jan 26, 2007
  6. 6/7 add logref support to git-symbolic-refNicolas Pitre, Jan 26, 2007
  7. 7/7 add reflog when moving HEAD to a new branchNicolas Pitre, Jan 26, 2007
  8. 2/3 create_symref(): do not assume pathname from git_path() persists long enoughJunio C Hamano, Jan 27, 2007

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.