----- On Mar 19, 2019, at 5:17 PM, Yannick Lamarre ylama...@efficios.com wrote:

> This removes the need to verify for eventless file descriptors and
> mitigates risks of bug due to behaviour mismatch.
> 
> Signed-off-by: Yannick Lamarre <ylama...@efficios.com>
> ---
> src/common/compat/compat-poll.c | 26 +++++++++++++++++++++++---
> 1 file changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/src/common/compat/compat-poll.c b/src/common/compat/compat-poll.c
> index b7c17df7..27e2002b 100644
> --- a/src/common/compat/compat-poll.c
> +++ b/src/common/compat/compat-poll.c
> @@ -292,6 +292,8 @@ error:
> int compat_poll_wait(struct lttng_poll_event *events, int timeout)
> {
>       int ret;
> +     int count = 0, idx;
> +     struct pollfd swapfd;

The declaration scope of swapfd should be narrower (see below).

> 
>       if (events == NULL || events->current.events == NULL) {
>               ERR("poll wait arguments error");
> @@ -324,10 +326,28 @@ int compat_poll_wait(struct lttng_poll_event *events, 
> int
> timeout)
>       }
> 
>       /*
> -      * poll() should always iterate on all FDs since we handle the pollset 
> in
> -      * user space and after poll returns, we have to try every fd for a 
> match.
> +      * Swap all nonzero revents pollfd structs to the beginning of the 
> array to
> +      * emulate cpmpat-epoll behaviour.

cpmpat -> compat

>        */
> -     return events->wait.nb_fd;
> +     if ( ret != events->wait.nb_fd && ret != 0) {

remove space after if (.

> +             while (count < ret && events->wait.events[count].revents != 0) {
> +                     count += 1;
> +             }
> +             idx = count + 1;
> +             while (count < ret) {
> +                     if (events->wait.events[idx].revents != 0) {

You should declare swapfd in this scope.

> +                             swapfd = events->wait.events[idx];
> +                             events->wait.events[idx] = 
> events->wait.events[count];
> +                             events->wait.events[count] = swapfd;
> +                             count += 1;
> +                     }
> +                     if (idx < (events->wait.nb_fd - 1)) {
> +                             idx += 1;
> +                     }
> +             }

I don't think this algorithm does what is written on the box. :-/

Thanks,

Mathieu


> +     }
> +
> +     return ret;
> 
> error:
>       return -1;
> --
> 2.11.0
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

Reply via email to