[PATCH] log -L: do not free parents lists we might need again

[Thomas Rast <trast@student.ethz.ch>]

The parent rewriting code of 'git log -L' was too aggressive in
freeing memory: assign_range_to_parent() will free the commit->parents
field when it sees that a parent cannot pass off any blame (is a root
commit in rewritten history).

Its caller assign_parents_range() however will, upon finding the first
parent that takes *full* blame for all ranges, rewind and reinstate
all previous parents' line ranges and parent lists.  This resurrects
pointers to ranges that were freed in assign_range_to_parent() under
some circumstances.

Furthermore, we must not empty the parent lists either: the same
rewind/reinstate code relies on them.

Do both only if the commit was an ordinary (not merge or root) commit,
in which case the merge code-path discussed here is never taken.

Reported-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
[sorry for the double post, forgot to cc the list]

After staring at it for some time, I think this is the problem.

Sadly I cannot come up with an independent test that reproduces it (or
at least generates a valgrind warning).  Based on my analysis I tried

 diff --git a/t/t4302-log-line-merge-history.sh b/t/t4302-log-line-merge-history.sh
 index 8634116..7c86903 100755
 --- a/t/t4302-log-line-merge-history.sh
 +++ b/t/t4302-log-line-merge-history.sh
 @@ -171,4 +171,17 @@ test_expect_success 'validate the graph output.' '
  	test_cmp current-graph expected-graph
  '
  
 +test_expect_success 'set up trivial side merge' '
 +	git checkout -b trivial-side &&
 +	echo new_line >> path0 &&
 +	git add path0 &&
 +	git commit -m new_line &&
 +	git checkout master &&
 +	git merge --no-ff trivial-side
 +'
 +
 +test_expect_success 'log -L on the trivial-merged file' '
 +	git log -L /new_line/,+1 path0
 +'
 +
  test_done

but that does not fail.  I am hesitant to add your original test
because it strays into the directory where git was built, to test with
git's own history.  It just feels wrong.


 line.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/line.c b/line.c
index 63dd19a..b0deafb 100644
--- a/line.c
+++ b/line.c
@@ -961,8 +961,10 @@ static int assign_range_to_parent(struct rev_info *rev, struct commit *c,
 		 * If there is no new ranges assigned to the parent,
 		 * we should mark it as a 'root' commit.
 		 */
-		free(c->parents);
-		c->parents = NULL;
+		if (c->parents && !c->parents->next) {
+			free(c->parents);
+			c->parents = NULL;
+		}
 	}
 
 	/* and the ranges of current commit c is updated */
-- 
1.7.3.rc1.333.gf62981