git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
To: git@vger.kernel.org
Cc: Stephen Boyd <sboyd@codeaurora.org>, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 4/7] unpack-trees: create working copy of merge entry in merged_entry
Date: Thu, 30 May 2013 13:34:21 +0200	[thread overview]
Message-ID: <1369913664-49734-5-git-send-email-rene.scharfe@lsrfire.ath.cx> (raw)
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

  parent reply	other threads:[~2013-05-30 11:34 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-30 11:34 [PATCH 0/7] unpack-trees: plug memory leak for merges René Scharfe
2013-05-30 11:34 ` [PATCH 1/7] cache: mark cache_entry pointers const René Scharfe
2013-05-30 11:34 ` [PATCH 2/7] read-cache: " René Scharfe
2013-05-30 11:34 ` [PATCH 3/7] unpack-trees: factor out dup_entry René Scharfe
2013-06-04 15:06   ` Peter Krefting
2013-05-30 11:34 ` René Scharfe [this message]
2013-05-30 11:34 ` [PATCH 5/7] diff-lib, read-tree, unpack-trees: mark cache_entry pointers const René Scharfe
2013-05-30 11:34 ` [PATCH 6/7] diff-lib, read-tree, unpack-trees: mark cache_entry array paramters const René Scharfe
2013-05-30 11:34 ` [PATCH 7/7] unpack-trees: free cache_entry array members for merges René Scharfe
2013-05-30 12:04   ` Felipe Contreras
2013-05-30 14:40     ` René Scharfe
2013-05-30 15:20       ` Felipe Contreras

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1369913664-49734-5-git-send-email-rene.scharfe@lsrfire.ath.cx \
    --to=rene.scharfe@lsrfire.ath.cx \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sboyd@codeaurora.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).