On Sun, 2 Dec 2007, Junio C Hamano wrote: > Daniel Barkalow <barkalow@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> > > --- > > 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* - 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