dbwrap_tool: Simplify persistent/non-persistent check / dbwrap_tool: Avoid an unnecessary "else"

Jeremy Allison jra at samba.org
Mon Sep 3 23:10:26 UTC 2018


On Tue, Sep 04, 2018 at 09:43:32AM +1200, Andrew Bartlett via samba-technical wrote:
> On Mon, 2018-09-03 at 23:30 +0200, Ralph Böhme wrote:
> > Howdy,
> > 
> > On Tue, Sep 04, 2018 at 09:11:32AM +1200, Andrew Bartlett wrote:
> > > While each of these looks reasonable on its own, is this really the
> > > clearest (rather than cleverest) possible way this could have been
> > > constructed as a whole?
> > 
> > s/clearest/clearer/
> > 
> > yes. The clearest way? Who knows. Patches welcome.
> > 
> > > > -       if ((persistent == 0 && non_persistent == 0) ||
> > > > -           (persistent == 1 && non_persistent == 1))
> > > > -       {
> > > > +       if ((persistent + non_persistent) != 1) {
> > > >                 d_fprintf(stderr, "ERROR: you must specify exactly one "
> > > >                           "of --persistent and --non-persistent\n");
> > > >                 goto done;
> > > > -       } else if (non_persistent == 1) {
> > > > +       }
> > > > +       if (non_persistent == 1) {
> > > >                 tdb_flags |= TDB_CLEAR_IF_FIRST;
> > > >         }
> > > 
> > > What specifically was the goal of the change here?
> > 
> > What specifically is the goal of this rhetorical question?
> 
> I was hoping to hear a rationale as to how this is 'simplified', to
> argue against such 'simplification' in the future and to rhetorically
> suggest that isn't the case here.

It's a matter of personal taste. IMHO, the new
code is a lot clearer to me.

It's hard for me to see exactly what:

	if ((persistent == 0 && non_persistent == 0) ||
	    (persistent == 1 && non_persistent == 1))

is trying to enforce, but:

	if ((persistent + non_persistent) != 1) {

is *much* clearer (either one of persistent or
non_persistent must be set to 1, but not both,
or neither).

I guess it just comes down to how your mind
works when looking at these things.





More information about the samba-technical mailing list