Re: [PATCH] fix segfault with git log -c --follow

From: Junio C Hamano <gitster@pobox.com>
Date: May 28, 2013, 17:22 UTC
Message-ID: <7vk3mj10l2.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130527224957.GA7492@ecki>

Clemens Buchacher <drizzd@aon.at> writes:

Show 7 quoted lines

> In diff_tree_combined we make a copy of diffopts. In
> try_to_follow_renames, called via diff_tree_sha1, we free and
> re-initialize diffopts->pathspec->items. Since we did not make a deep
> copy of diffopts in diff_tree_combined, the original diffopts does not
> get the update. By the time we return from diff_tree_combined,
> rev->diffopt->pathspec->items points to an invalid memory address. We
> get a segfault next time we try to access that pathspec.

I am not quite sure if I follow.  Do you mean

	diff_tree_combined()
        - makes a shallow copy of rev->diffopt
        - calls diff_tree_sha1()
          diff_tree_sha1()
          - tries to follow rename and clobbers diffopt
        - tries to use the shallow copy of original rev->diffopt
          that no longer is valid, which is a problem

I wonder, just like we force recursive and disable external on the copy before we use it to call diff_tree_sha1(), if we should disable follow-renames on it. "--follow" is an option that is given to the history traversal part and it should not play any role in getting the pairwise diff with all parents diff_tree_combined() does.

Besides,

 - "--follow" hack lets us keep track of only one path; and

 - "-c" and "--cc" make sense only when dealing with a merge commit
   and the path in the child may have come from different path in
   parents,

so I am not sure if allowing combination of "--follow -c/--cc" makes much sense in the first place.