On Tue, Mar 13, 2012 at 11:11:23AM -0700, Junio C Hamano wrote:
> 徐迪 <xudifsd@xxxxxxxxx> writes:
>
> > The sigchain APIs require user to code like:
> >
> > void clean_foo_on_signal(int sig)
> > {
> > clean_foo();
> > sigchain_pop(sig);
> > raise(sig);
> > }
>
> I can see the repetition, but I do not think your macro is a very good way
> to reduce it. Can't we fix the sigchain API a bit, perhaps first by
> making a bit more state available to the sigchain stack?
>
> typedef void (*sigchain_fn)(int, void *);
> int sigchain_push(int sig, sigchain_fn fn, void *cb_data);
> int sigchain_pop(int sig);
> void sigchain_push_common(sigchain_fn fn, void *cb_data);
FWIW, when I wrote the original sigchain implementation I considered
doing something more complex like this. The calling code ends up
shorter, but I wonder if the end result is really any easier to
understand. Ditto for the macro. It's shorter, but much harder for a
reader to understand. The boilerplate, while ugly, reads very simply,
and is not error-prone.
So I don't like boilerplate or repetition, but this may be one of those
times where it is simply more clear to just be verbose.
> *1* Yes, I know, this casts between a data pointer and a function pointer
> that is not portable, but the purpose of this pseudo-code is primarily to
> illustrate the high level view of the idea. We would probably want to be
> able to pass callback value to the clean_foo() function and at that point,
> we would likely to be passing a pointer to a struct, and we could declare
> the first element of such a struct is a pointer to a sigchain_fn, or
> something.
That is the correct way to do it, but where does the struct memory come
from? You'd have to allocate it on the heap, unless you want to burden
the caller.
-Peff
--
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