Re: [PATCH v5 16/16] merge-recursive: flush output buffer even when erroring out

Johannes Schindelin [off-list ref] writes:

This is of course a good change, but we need to assume that no
further output is made from the remainder of the function for the
change in the next hunk to remove the existing flush to be correct.

And once we assume that, then the "we no longer need this buffer, so
release it" added in 15/16 can also move here, right?

I am wondering if there is a low-impact way to make sure that
assumption will not be broken.

Hi Junio,

On Wed, 27 Jul 2016, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

diff --git a/merge-recursive.c b/merge-recursive.c
index a16b150..66e93e0 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c

@@ -2069,6 +2069,7 @@ int merge_recursive(struct merge_options *o,
 	o->ancestor = "merged common ancestors";
 	clean = merge_trees(o, h1->tree, h2->tree, merged_common_ancestors->tree,
 			    &mrtree);
+	flush_output(o);
 	if (clean < 0)
 		return clean;

This is of course a good change, but we need to assume that no
further output is made from the remainder of the function for the
change in the next hunk to remove the existing flush to be correct.

Please note that nothing prevents the code further down from adding more
output. All we do here is flushing the output *so far*, in case we return
an error. And of course nothing gets flushed if buffer_output == 2,
because that value states that the caller wants to take care of displaying
the output herself.

But you made me realize that I cannot simply *move* the flush_output()
call here, in case that code in between will eventually add output.

Thanks,
Dscho