Re: [PATCH 4/9] merge_recursive: abort properly upon errors

> Johannes Schindelin <johannes.schindelin@gmx.de> 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@gmx.de>
> > ---
> >  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.