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

[PATCH 09/21] remote: make match_refs() copy src ref before assigning to peer_ref

From
Jay Soffian <jaysoffian@gmail.com>
Date
Feb 25, 2009, 08:32 UTC
Message-ID
<4765629ca76c4268d8ba2625f1adab27be838ca1.1235546708.git.jaysoffian@gmail.com>
In-Reply-To
<cover.1235546707.git.jaysoffian@gmail.com>

In some instances, match_refs() sets the peer_ref field of refs in the dst list such that it points to a ref in the src list. This prevents callers from freeing both the src and dst lists, as doing so would cause a double-free since free_refs() frees the peer_ref.

As well, the following configuration causes two refs in the dst list to have the same peer_ref, which can also lead to a double-free:

  push = refs/heads/master:refs/heads/backup
  push = refs/heads/master:refs/heads/master

Existing callers of match_heads() call it only once and then terminate, w/o ever bothering to free the src or dst lists, so this is not currently a problem.

This patch modifies match_refs() to first copy any refs it plucks from the src list before assigning them as a peer_ref. This allows builtin-remote, a future caller, to free the src and dst lists.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 remote.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/remote.c b/remote.c
index 3940a3c..81def8b 100644
--- a/remote.c
+++ b/remote.c
@@ -928,6 +928,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
 			  struct refspec *rs)
 {
 	struct ref *matched_src, *matched_dst;
+	int copy_src;
 
 	const char *dst_value = rs->dst;
 	char *dst_guess;
@@ -938,6 +939,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	matched_src = matched_dst = NULL;
 	switch (count_refspec_match(rs->src, src, &matched_src)) {
 	case 1:
+		copy_src = 1;
 		break;
 	case 0:
 		/* The source could be in the get_sha1() format
@@ -947,6 +949,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
 		matched_src = try_explicit_object_name(rs->src);
 		if (!matched_src)
 			return error("src refspec %s does not match any.", rs->src);
+		copy_src = 0;
 		break;
 	default:
 		return error("src refspec %s matches more than one.", rs->src);
@@ -992,7 +995,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
 		return error("dst ref %s receives from more than one src.",
 		      matched_dst->name);
 	else {
-		matched_dst->peer_ref = matched_src;
+		matched_dst->peer_ref = copy_src ? copy_ref(matched_src) : matched_src;
 		matched_dst->force = rs->force;
 	}
 	return 0;
@@ -1100,7 +1103,7 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
 			dst_peer = make_linked_ref(dst_name, dst_tail);
 			hashcpy(dst_peer->new_sha1, src->new_sha1);
 		}
-		dst_peer->peer_ref = src;
+		dst_peer->peer_ref = copy_ref(src);
 		dst_peer->force = pat->force;
 	free_name:
 		free(dst_name);
-- 
1.6.2.rc1.291.g83eb
Previous: Jeff KingNext: Jay Soffian
Message 14 of 28 in “git remote: set-head and new show output”
  1. 00/21 git remote: set-head and new show outputJay Soffian, Feb 25, 2009
  2. 01/21 test scripts: refactor start_httpd helperJay Soffian, Feb 25, 2009
  3. 02/21 add basic http clone/fetch testsJay Soffian, Feb 25, 2009
  4. 03/21 refactor find_ref_by_name() to accept const listJay Soffian, Feb 25, 2009
  5. 04/21 move duplicated get_local_heads() to remote.cJay Soffian, Feb 25, 2009
  6. 05/21 move duplicated ref_newer() to remote.cJay Soffian, Feb 25, 2009
  7. 06/21 move locate_head() to remote.cJay Soffian, Feb 25, 2009
  8. 07/21 remote: simplify guess_remote_head()Jay Soffian, Feb 25, 2009
  9. 08/21 remote: let guess_remote_head() optionally return all matchesJay Soffian, Feb 25, 2009
  10. Jeff KingFeb 26, 2009
  11. Jeff KingFeb 26, 2009
  12. Jay SoffianFeb 26, 2009
  13. Jeff KingFeb 27, 2009
  14. 09/21 remote: make match_refs() copy src ref before assigning to peer_refJay Soffian, Feb 25, 2009
  15. 10/21 remote: make match_refs() not short-circuitJay Soffian, Feb 25, 2009
  16. 11/21 string-list: new for_each_string_list() functionJay Soffian, Feb 25, 2009
  17. 12/21 builtin-remote: refactor duplicated cleanup codeJay Soffian, Feb 25, 2009
  18. 13/21 builtin-remote: remove unused code in get_ref_statesJay Soffian, Feb 25, 2009
  19. 14/21 builtin-remote: rename variables and eliminate redundant function callJay Soffian, Feb 25, 2009
  20. 15/21 builtin-remote: make get_remote_ref_states() always populate states.trackedJay Soffian, Feb 25, 2009
  21. 16/21 builtin-remote: fix two inconsistencies in the output of "show <remote>"Jay Soffian, Feb 25, 2009
  22. 17/21 builtin-remote: teach show to display remote HEADJay Soffian, Feb 25, 2009
  23. 18/21 builtin-remote: add set-head subcommandJay Soffian, Feb 25, 2009
  24. 19/21 remote: make guess_remote_head() use exact HEAD lookup if it is availableJay Soffian, Feb 25, 2009
  25. 20/21 builtin-remote: new show output styleJay Soffian, Feb 25, 2009
  26. 21/21 builtin-remote: new show output style for push refspecsJay Soffian, Feb 25, 2009
  27. Jeff KingFeb 26, 2009
  28. Junio C HamanoFeb 26, 2009

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.