On Fri, 22 Mar 2019 at 11:04, Mathieu Desnoyers <mathieu.desnoy...@efficios.com> wrote: > > ----- On Mar 21, 2019, at 4:25 PM, Jeremie Galarneau > jeremie.galarn...@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 _______________________________________________ lttng-dev mailing list lttng-dev@lists.lttng.org https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
