merge-recursive: avoid losing output and leaking memory holding that output

If opt->buffer_output is less than 2, then merge_trees(),
merge_recursive(), and merge_recursive_generic() are all supposed to
flush the opt->obuf output buffer to stdout and release any memory it
holds.  merge_trees() did not do this.  Move the logic that handles this
for merge_recursive_internal() to merge_finalize() so that all three
methods handle this requirement.

Note that this bug didn't cause any problems currently, because there
are only two callers of merge_trees() right now (a git grep for
'merge_trees(' is misleading because builtin/merge-tree.c also defines a
'merge_tree' function that is unrelated), and only one of those is
called with buffer_output less than 2 (builtin/checkout.c), but it set
opt->verbosity to 0, for which there is only currently one non-error
message that would be shown: "Already up to date!".  However, that one
message can only occur when the merge is utterly trivial (the merge base
tree exactly matches the merge tree), and builtin/checkout.c already
attempts a trivial merge via unpack_trees() before falling back to
merge_trees().

Also, if opt->buffer_output is 2, then the caller is responsible to
handle showing any output in opt->obuf and for free'ing it.  This
requirement might be easy to overlook, so add a comment to
merge-recursive.h pointing it out.  (There are currently two callers
that set buffer_output to 2, both in sequencer.c, and both of which
handle this correctly.)

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/merge-recursive.c b/merge-recursive.c
index 43dec3307e8a2512895581eb1ece4567d47f8052..262db8bebbb887624eabb8afaea8f1b60a54d8f0 100644 (file)
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -3598,9 +3598,6 @@ static int merge_recursive_internal(struct merge_options *opt,
                commit_list_insert(h1, &(*result)->parents);
                commit_list_insert(h2, &(*result)->parents->next);
        }
-       flush_output(opt);
-       if (!opt->call_depth && opt->buffer_output < 2)
-               strbuf_release(&opt->obuf);
        return clean;
 }
 
@@ -3620,6 +3617,9 @@ static int merge_start(struct merge_options *opt, struct tree *head)
 
 static void merge_finalize(struct merge_options *opt)
 {
+       flush_output(opt);
+       if (!opt->call_depth && opt->buffer_output < 2)
+               strbuf_release(&opt->obuf);
        if (show(opt, 2))
                diff_warn_rename_limit("merge.renamelimit",
                                       opt->needed_rename_limit, 0);

diff --git a/merge-recursive.h b/merge-recursive.h
index 9e040608fec98e09ac8fa01abc58ed724125735a..933d6e76427d217c30d51299b665981a127c992b 100644 (file)
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -38,7 +38,8 @@ struct merge_options {
        /* console output related options */
        int verbosity;
        unsigned buffer_output; /* 1: output at end, 2: keep buffered */
-       struct strbuf obuf;     /* output buffer */
+       struct strbuf obuf;     /* output buffer; if buffer_output == 2, caller
+                                * must handle and call strbuf_release */
 
        /* miscellaneous control options */
        const char *subtree_shift;