Skip to content

Commit e95e481

Browse files
newrengitster
authored andcommitted
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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent a779fb8 commit e95e481

File tree

2 files changed

+5
-4
lines changed

2 files changed

+5
-4
lines changed

merge-recursive.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3598,9 +3598,6 @@ static int merge_recursive_internal(struct merge_options *opt,
35983598
commit_list_insert(h1, &(*result)->parents);
35993599
commit_list_insert(h2, &(*result)->parents->next);
36003600
}
3601-
flush_output(opt);
3602-
if (!opt->call_depth && opt->buffer_output < 2)
3603-
strbuf_release(&opt->obuf);
36043601
return clean;
36053602
}
36063603

@@ -3620,6 +3617,9 @@ static int merge_start(struct merge_options *opt, struct tree *head)
36203617

36213618
static void merge_finalize(struct merge_options *opt)
36223619
{
3620+
flush_output(opt);
3621+
if (!opt->call_depth && opt->buffer_output < 2)
3622+
strbuf_release(&opt->obuf);
36233623
if (show(opt, 2))
36243624
diff_warn_rename_limit("merge.renamelimit",
36253625
opt->needed_rename_limit, 0);

merge-recursive.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ struct merge_options {
3838
/* console output related options */
3939
int verbosity;
4040
unsigned buffer_output; /* 1: output at end, 2: keep buffered */
41-
struct strbuf obuf; /* output buffer */
41+
struct strbuf obuf; /* output buffer; if buffer_output == 2, caller
42+
* must handle and call strbuf_release */
4243

4344
/* miscellaneous control options */
4445
const char *subtree_shift;

0 commit comments

Comments
 (0)