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

[PATCH 4/7] unpack-trees: create working copy of merge entry in merged_entry

From
René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date
May 30, 2013, 11:34 UTC
Message-ID
<1369913664-49734-5-git-send-email-rene.scharfe@lsrfire.ath.cx>
In-Reply-To
<1369913664-49734-1-git-send-email-rene.scharfe@lsrfire.ath.cx>

Duplicate the merge entry right away and work with that instead of modifying the entry we got and duplicating it only at the end of the function. Then mark that pointer const to document that we don't modify the referenced cache_entry.

This change is safe because all existing merge functions call merged_entry just before returning (or not at all), i.e. they don't care about changes to the referenced cache_entry after the call. unpack_nondirectories and unpack_index_entry, which call the merge functions through call_unpack_fn, aren't interested in such changes neither.

The change complicates merged_entry a bit because we have to free the copy if we error out, but allows callers to pass a const pointer.

Signed-off-by: René Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 unpack-trees.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/unpack-trees.c b/unpack-trees.c
index e8b4cc1..2fecef8 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1466,10 +1466,12 @@ static int verify_absent_sparse(struct cache_entry *ce,
 	return verify_absent_1(ce, orphaned_error, o);
 }
 
-static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
-		struct unpack_trees_options *o)
+static int merged_entry(const struct cache_entry *ce,
+			struct cache_entry *old,
+			struct unpack_trees_options *o)
 {
 	int update = CE_UPDATE;
+	struct cache_entry *merge = dup_entry(ce);
 
 	if (!old) {
 		/*
@@ -1487,8 +1489,11 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
 		update |= CE_ADDED;
 		merge->ce_flags |= CE_NEW_SKIP_WORKTREE;
 
-		if (verify_absent(merge, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o))
+		if (verify_absent(merge,
+				  ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o)) {
+			free(merge);
 			return -1;
+		}
 		invalidate_ce_path(merge, o);
 	} else if (!(old->ce_flags & CE_CONFLICTED)) {
 		/*
@@ -1502,8 +1507,10 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
 			copy_cache_entry(merge, old);
 			update = 0;
 		} else {
-			if (verify_uptodate(old, o))
+			if (verify_uptodate(old, o)) {
+				free(merge);
 				return -1;
+			}
 			/* Migrate old flags over */
 			update |= old->ce_flags & (CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);
 			invalidate_ce_path(old, o);
@@ -1516,7 +1523,7 @@ static int merged_entry(struct cache_entry *merge, struct cache_entry *old,
 		invalidate_ce_path(old, o);
 	}
 
-	add_entry(o, merge, update, CE_STAGEMASK);
+	do_add_entry(o, merge, update, CE_STAGEMASK);
 	return 1;
 }
 
-- 
1.8.3
Previous: Peter KreftingNext: René Scharfe
Message 6 of 12 in “unpack-trees: plug memory leak for merges”
  1. 0/7 unpack-trees: plug memory leak for mergesRené Scharfe, May 30, 2013
  2. 1/7 cache: mark cache_entry pointers constRené Scharfe, May 30, 2013
  3. 2/7 read-cache: mark cache_entry pointers constRené Scharfe, May 30, 2013
  4. 3/7 unpack-trees: factor out dup_entryRené Scharfe, May 30, 2013
  5. Peter KreftingJun 4, 2013
  6. 4/7 unpack-trees: create working copy of merge entry in merged_entryRené Scharfe, May 30, 2013
  7. 5/7 diff-lib, read-tree, unpack-trees: mark cache_entry pointers constRené Scharfe, May 30, 2013
  8. 6/7 diff-lib, read-tree, unpack-trees: mark cache_entry array paramters constRené Scharfe, May 30, 2013
  9. 7/7 unpack-trees: free cache_entry array members for mergesRené Scharfe, May 30, 2013
  10. Felipe ContrerasMay 30, 2013
  11. René ScharfeMay 30, 2013
  12. Felipe ContrerasMay 30, 2013

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.