Re: [RFC/PATCH 02/18] revert: change many die() calls into "return error()" calls

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Nov 26, 2010, 06:05 UTC
Message-ID: <20101126060543.GC18751@burratino>
In-Reply-To: <20101125212050.5188.21758.chriscool@tuxfamily.org>

Christian Couder wrote:

Show 8 quoted lines

> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -280,16 +280,18 @@ static struct tree *empty_tree(void)
>  	return tree;
>  }
>  
> -static NORETURN void die_dirty_index(const char *me)
> +static int error_dirty_index(const char *me)

In general sounds to me like a good thing to do. But for your use case (writing out TODO and DONE files when cherry-pick fails), wouldn't a set_die_routine() also work?

I am tempted to suggest a series in the following order:

 1. set die routine with the desired behavior
 2. change die() calls to return error() so the nice stack unwinding
    takes place automatically (this should help with other libification
    work, anyway)

then maybe:

 3. add an assert(0) to die routine (perhaps protected by a compile-time
    option) so missing die() calls can be noticed
 4. remove the die routine once it is clear all problematic die calls
    have been eliminated.

... but wait: would all such die calls ever be eliminated? xmalloc, xmkstemp, and similar functions are perhaps too convenient to avoid. So it might be simpler to stick to (1) and treat (2) as a separate topic.