Re: [PATCH v4 06/16] merge_recursive: abort properly upon errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Junio,

On Mon, 25 Jul 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@xxxxxx> writes:
> 
> > There are a couple of places where return values indicating errors
> > are ignored. Let's teach them manners.
> 
> That is because the return value never indicated errors before this
> series, isn't it?  A true error used to be expressed by dying, and
> the return value indicating "cleanliness" of the merge were
> deliberately ignored.
> 
> The world order changed by previous patches in this series and the
> callers need to be updated to take the new kind of return values
> into account.  That is not teaching them manners ;-)

I reworded that commit message.

> > 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 dc3182b..2d4cb80 100644
> > --- a/merge-recursive.c
> > +++ b/merge-recursive.c
> > @@ -1949,8 +1949,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--;
> 
> This hunk feels somewhat wrong as-is.
> 
> There is a comment before the pre-context explaining why cleanness
> flag is ignored.  It needs to be updated.  We still do not care
> about cleanliness, i.e. 0=clean, 1=merged with conflict, but we now
> can get negative values so we need to reject and return early if
> this call indicates an error.

I updated that comment. Hopefully now the hunk makes sense ;-)

Ciao,
Dscho
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]