Conditions always true in tevent_standard.c
jra at samba.org
Mon Jul 11 23:59:07 UTC 2016
On Mon, Jul 04, 2016 at 07:17:31PM +0200, Ralph Boehme wrote:
> On Mon, Jul 04, 2016 at 07:17:20PM +0300, Nikolai Kondrashov wrote:
> > On 06/30/2016 10:24 PM, Jeremy Allison wrote:
> > >On Wed, Jun 29, 2016 at 03:43:34PM +0300, Nikolai Kondrashov wrote:
> > >>Please find the patch attached. I also tried to rearrange the conditions to
> > >>make the code easier to understand.
> > >
> > >I like the comment. I don't like the rearrangement of conditions.
> > >
> > >In Samba we prefer simplified if statement that cause early exit,
> > >rather than complex if conditions. Unfortunately your patch makes
> > >the if conditions complex.
> > >
> > >I'd prefer something that just adds the comment before the
> > >if == NULL check.
> > I understand, and this sounds like a good convention in general.
> > However, I think it makes these small functions and conditions harder to read.
> > Perhaps an exception for small functions would be in order.
> > Regardless, it's your code and you decide, so here is an updated patch :)
> I know I'm late in the game and didn't check all the nitty-gritty
> details, but I'm wondering if it's a good idea to misuse the functions
> pointers as state variable. Maybe adding something like bool
> epoll_panicked is a better idea?
Perfect is the enemy of the good :-). Let's get this comment
fix in first so people don't keep getting confused, then look
at (a) tidying up with your bool idea and the (b) removing
the fallback code entirely now that we almost certainly don't
need it on Linux.
More information about the samba-technical