[PATCH] ctdb: simplify logic (Re: [PATCH] Fixes related to ctdb command line option --nosetsched)

Amitay Isaacs amitay at gmail.com
Mon Jun 20 14:39:36 UTC 2016


On Tue, Jun 21, 2016 at 12:29 AM, Michael Adam <obnox at samba.org> wrote:

> On 2016-06-21 at 00:13 +1000, Amitay Isaacs wrote:
> > On Mon, Jun 20, 2016 at 7:39 PM, Michael Adam <obnox at samba.org> wrote:
> >
> > > Hi,
> > >
> > > attached find a follow-up patch to this patchset that
> > > simplifies a bool assignment. Imho it reads more
> > > naturally like this.
> > >
> > > Review appreciated!
> > >
> > > Michael
> > >
> >
> > I prefer the long form when multiple conditions are involved.  It makes
> > easier for me to parse and make less mistakes.  I don't think expressions
> > with double negatives read naturally.
>
> Hmm, it depends. Sometimes !(a || b) reads more natuarally
> and sometimes (!a && !b) does. But this was not my point:
> I found it rather unnatural to read as is, but rather due to the
> if (..) { x = true; } else { x = false; } pattern and
> looking to turn that into one assignment.
>
> > But if you really free strongly
> > about the patch, someone else can review and push it.
>
> No need, but how about this:
>
>         ctdb->do_setsched = (options.nosetsched != 1);
>         if (ctdb->valgrinding) {
>                 ctdb->do_setsched = false;
>         }
>
> That makes it clear what is the rule
> and what is the exception.
>

This one is definitely much better. Thanks. :-)

Reviewed-by: me

If you also dislike this, I'll drop the request. :-)
>
> Cheers - Michael
>

Amitay.


More information about the samba-technical mailing list