Conditions always true in tevent_standard.c

[Nikolai Kondrashov]

On 06/28/2016 08:30 PM, Jeremy Allison wrote:
> On Tue, Jun 28, 2016 at 02:07:10PM +0300, Nikolai Kondrashov wrote:
>> Hi everyone,
>>
>> I was reading about tevent and browsing the code, when I noticed these two
>> conditions in tevent_standard.c:
>>
>>      ret = glue->epoll_ops->loop_once(ev, location);
>>      if (glue->epoll_ops != NULL) {
>>          /* No fallback */
>>          return ret;
>>      }
>>
>> and
>>
>>      ret = glue->epoll_ops->loop_wait(ev, location);
>>      if (glue->epoll_ops != NULL) {
>>          /* No fallback */
>>          return ret;
>>      }
>>
>> To me it seems they are never going to be false (the program would segfault
>> before they are reached if glue->epoll_ops is NULL) and I suspect there's some
>> logic bug here. However, unfortunately, I don't have time to figure out what
>> exactly should be done about this, so I'll leave it up to you.
>
> No, it isn't a logic bug (I had to explain the same thing to a
> Google engineer). The epoll backend can fall back on an epoll
> syscall failure (early versions of Linux might have had problems
> here) so check out the code in tevent_standard.c:
>
> #ifdef HAVE_EPOLL
>                 tevent_epoll_set_panic_fallback(ev, std_fallback_to_poll);
> #endif

Yeah, I understood that much.

>> Still, if there's some purpose to that, then I think a comment is in order.
>
> Please submit a comment as a git-am patch, once you grok the code :-).

Ah, I see now. That was unexpected :)

Please find the patch attached. I also tried to rearrange the conditions to
make the code easier to understand.

Nick
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-tevent-Clarify-apparently-useless-conditions.patch
Type: text/x-patch
Size: 2801 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160629/e9cf88fa/0001-tevent-Clarify-apparently-useless-conditions.bin>