Re: [PATCH 1/2] Allow callers of unpack_trees() to handle failure

From: Daniel Barkalow <barkalow@iabervon.org>
Date: Dec 3, 2007, 06:31 UTC
Message-ID: <Pine.LNX.4.64.0712030126390.5349@iabervon.org>
In-Reply-To: <7v63zgqorw.fsf@gitster.siamese.dyndns.org>

On Sun, 2 Dec 2007, Junio C Hamano wrote:

Show 27 quoted lines

> Daniel Barkalow <barkalow@iabervon.org> writes:
> 
> > Return an error from unpack_trees() instead of calling die(), and exit
> > with an error in read-tree. The merge function can return negative to
> > abort.
> >
> > This will be used in builtin-checkout -m.
> >
> > Signed-off-by: Daniel Barkalow <barkalow@iabervon.org>
> > ---
> >  builtin-read-tree.c |    3 +-
> >  unpack-trees.c      |   85 ++++++++++++++++++++++++++++----------------------
> >  2 files changed, 50 insertions(+), 38 deletions(-)
> >
> > diff --git a/builtin-read-tree.c b/builtin-read-tree.c
> > index 43cd56a..4f680c3 100644
> > --- a/builtin-read-tree.c
> > +++ b/builtin-read-tree.c
> > @@ -269,7 +269,8 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
> >  		parse_tree(tree);
> >  		init_tree_desc(t+i, tree->buffer, tree->size);
> >  	}
> > -	unpack_trees(nr_trees, t, &opts);
> > +	if (unpack_trees(nr_trees, t, &opts))
> > +		return 128;
> 
> Magic 128 when everybody else in the patch returns -1 for error?

This is a cmd_ function, so this is the exit code. 128 is what die() uses. Should this be explicitly an exit(128)?

> Otherwise the changes seem sensible, as long as the callers are paying
> attention to the return values, which I admit that I did not check.

The only callers are here and merge-recursive, and merge-recursive was already checking the return value (and never getting anything); merge-recursive die()s if it gets non-zero.

	-Daniel
*This .sig left intentionally blank*