Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> There are a couple of places where return values indicating errors
> are ignored. Let's teach them manners.
>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> ---
> merge-recursive.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/merge-recursive.c b/merge-recursive.c
> index bcb53f0..c4ece96 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1944,8 +1944,9 @@ int merge_recursive(struct merge_options *o,
> saved_b2 = o->branch2;
> o->branch1 = "Temporary merge branch 1";
> o->branch2 = "Temporary merge branch 2";
> - merge_recursive(o, merged_common_ancestors, iter->item,
> - NULL, &merged_common_ancestors);
> + if (merge_recursive(o, merged_common_ancestors, iter->item,
> + NULL, &merged_common_ancestors) < 0)
> + return -1;
> o->branch1 = saved_b1;
> o->branch2 = saved_b2;
> o->call_depth--;
OK, this early return (and others in this patch) are only for
negative (i.e. error) cases, and "attempted a merge, resulted in
conflicts" cases are handled as before.
Which is good.
I wonder if o->branch[12] need to be restored, though. The only
sensible thing the caller can do is to punt, but would it expect to
be able to do some error reporting based on these fields, e.g.
printf("merge of %s and %s failed", o->branch1, o->branch2) or
something? In addition to that kind of "state restoration", we may
need to watch out for resource leaks, but I think there is none at
least these three early returns.
Thanks.
> @@ -1961,6 +1962,8 @@ 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);
> + if (clean < 0)
> + return clean;
>
> if (o->call_depth) {
> *result = make_virtual_commit(mrtree, "merged tree");
> @@ -2017,6 +2020,9 @@ int merge_recursive_generic(struct merge_options *o,
> hold_locked_index(lock, 1);
> clean = merge_recursive(o, head_commit, next_commit, ca,
> result);
> + if (clean < 0)
> + return clean;
> +
> if (active_cache_changed &&
> write_locked_index(&the_index, lock, COMMIT_LOCK))
> return error(_("Unable to write index."));
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html