[Libevent-users] [PATCH] signal.c, evsignal.h: properly save/restore previous signal handlers and fix memory stomp

[Christopher Layne]

On Fri, Nov 09, 2007 at 04:03:46PM -0800, Scott Lamb wrote:
> Christopher Layne wrote:
> >+	/* save previous handler setup */
> >+	if (sigaction(evsignal, NULL, sig->sa_old[evsignal]) == -1
> >+		|| sigaction(evsignal, &sa, NULL) == -1)
> 
> Not worth changing unless you're redoing the patch anyway, but is there 
> some reason you aren't doing this in a single call? I.e.,
> 
>     if (sigaction(evsignal, &sa, sig->sa_old[evsignal]) == -1) {

Good idea. I'll change it and resubmit it with the regress.c patch
also included (that Nick mentioned).

> ...
> 
> >-	if (!base->sig.ev_signal_added) {
> >-		base->sig.ev_signal_added = 1;
> >-		event_add(&base->sig.ev_signal, NULL);
> >+	if (!sig->ev_signal_added) {
> >+		sig->ev_signal_added = 1;
> >+		event_add(&sig->ev_signal, NULL);
> > 	}
> 
> There's a bug here (that predates your change): this code should handle 
> event_add() failure. (E.g., epoll_ctl() returning ENOMEM.)

Fix in same, or sweep up in a later patch? How many other places are
there where we're not currently checking the return value of
event_add()? If there are more than this, we might as well just do it
separately.

-cl