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
_______________________________________________
Libevent-users mailing list
Libevent-users@monkey.org
http://monkeymail.org/mailman/listinfo/libevent-users
