[lttng-dev] [PATCH lttng-tools v2] Change lttng_poll_wait behaviour of compat-poll to match compat-epoll

[Jérémie Galarneau]

On Fri, 22 Mar 2019 at 11:04, Mathieu Desnoyers
<mathieu.desnoyers at efficios.com> wrote:
>
> ----- On Mar 21, 2019, at 4:25 PM, Jeremie Galarneau jeremie.galarneau at efficios.com wrote:
>
> [...]
> >> +               assert(idx < events->wait.nb_fd);
> >
> > Not needed. Using assert() is fine to highlight assumptions across
> > interfaces/functions and/or catch catastrophic internal errors. Here,
> > this would be a very local condition that seems to be already
> > enforced.
> >
> > Reworking the 'for' condition to use 'i < events->wait.nb_fd' and
> > breaking out early of the loop is also an option here.
>
> The reason for using assert() here is to ensure that there is no discrepancy
> between the number of active poll fd in the set and the value returned by
> poll indicating that same number of active fds.
>
> We'd need a kernel bug (or memory corruption) to have a mismatch, but I thought
> having an assert in there would not hurt, since it might help diagnose issues
> that cross user-kernel boundaries.
>
> Thoughts ?

Hi,

I understand the intent, but I don't feel these type of kernel tests/checks
belong in the code.

However, if there is a specific concern that our use of poll() could highlight
this type of bug (in the kernel or in the interactions with lttng-modules),
we can add tests for that.

Thanks,
Jérémie

>
> Mathieu
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com


--
Jérémie Galarneau
EfficiOS Inc.
http://www.efficios.com