----- On Mar 20, 2019, at 3:42 PM, Mathieu Desnoyers
mathieu.desnoy...@efficios.com wrote:
> ----- 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. :-/
Based on our in-person discussion, this algorithm seems to work, but
would benefit from clearer variable names, and comments. Let's continue
the discussion on a v2.
Thanks,
Mathieu
>
> 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
--
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
