On Wed, 20.08.14 17:28, Tom Gundersen (t...@jklm.no) wrote:
> +
> +_public_ int sd_event_wait(sd_event *e, uint64_t timeout) {
> + struct epoll_event *ev_queue;
> + unsigned ev_queue_max;
> + int r, m, i;
> +
> + assert_return(e, -EINVAL);
> + assert_return(!event_pid_changed(e), -ECHILD);
> + assert_return(e->state != SD_EVENT_FINISHED, -ESTALE);
> + assert_return(e->state == SD_EVENT_PREPARED, -EBUSY);
> +
> + if (e->exit_requested) {
> + e->state = SD_EVENT_PENDING;
> + return 1;
> + }
>
> ev_queue_max = CLAMP(e->n_sources, 1U, EPOLL_QUEUE_MAX);
> ev_queue = newa(struct epoll_event, ev_queue_max);
> @@ -2262,12 +2280,10 @@ _public_ int sd_event_run(sd_event *e, uint64_t
> timeout) {
> m = epoll_wait(e->epoll_fd, ev_queue, ev_queue_max,
> timeout == (uint64_t) -1 ? -1 : (int) ((timeout +
> USEC_PER_MSEC - 1) / USEC_PER_MSEC));
> if (m < 0) {
> - r = errno == EAGAIN || errno == EINTR ? 1 : -errno;
> + r = -errno;
Why this change? We should still eat up EAGAIN/EINTR, they aren't really
errors...
> +_public_ int sd_event_run(sd_event *e, uint64_t timeout) {
> + int r;
> +
> + assert_return(e, -EINVAL);
> + assert_return(!event_pid_changed(e), -ECHILD);
> + assert_return(e->state != SD_EVENT_FINISHED, -ESTALE);
> + assert_return(e->state == SD_EVENT_PASSIVE, -EBUSY);
> +
> + r = sd_event_prepare(e);
> + if (r > 0)
> + return sd_event_dispatch(e);
> + if (r == 0) {
> + r = sd_event_wait(e, timeout);
> + if (r > 0)
> + return sd_event_dispatch(e);
> + else
> + return (r == -EAGAIN || r == -EINTR) ? 1 : r;
> + } else
> + return (r == -EAGAIN || r == -EINTR) ? 1 : r;
> +}
I think this would be nicer:
r = sd_event_prepare(...);
if (r < 0)
return ...
if (r > 0)
return ...
r = sd_event_wait(...)
if (r < 0)
return ...
if (r > 0)
return ...
return 0;
Looks good otherwise. I like it.
Lennart
--
Lennart Poettering, Red Hat
_______________________________________________
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
