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

Rowland Penny rpenny at samba.org
Tue Sep 4 09:33:06 UTC 2018


On Tue, 4 Sep 2018 11:23:44 +0200
Volker Lendecke via samba-technical <samba-technical at lists.samba.org>
wrote:

> On Tue, Sep 04, 2018 at 09:07:30PM +1200, Andrew Bartlett via
> samba-technical wrote:
> > On Tue, 2018-09-04 at 10:53 +0200, Volker Lendecke wrote:
> > > On Tue, Sep 04, 2018 at 10:49:40AM +0200, Ralph Böhme wrote:
> > > > 
> > > > On Tue, Sep 04, 2018 at 10:33:16AM +0200, Volker Lendecke wrote:
> > > > > 
> > > > > Enough is enough. Sorry for the trouble this caused.
> > > > > 
> > > > > Review appreciated!
> > > > can we please not revert this and keep the improved code? At
> > > > least three of
> > > > us (Jeremey, you, me) agree that the patches are an improvement.
> > > Please revert this. It caused a discussion that is really not
> > > worth it. My life does not depend on such controversial patches,
> > > and it is better to bury it sooner rather than later if it
> > > offends people so much. The former code worked and was much
> > > cleaner to many people, so it's better to restore the
> > > non-controversial state than to keep people
> > > upset.
> > 
> > G'Day Volker,
> > 
> > Please be willing to accept comment on your code, especially wrong,
> > opinionated and ill-considered comment.  
> 
> Sure. But the pure fact that so many people felt inclined to comment
> tells me that I should never have touched that piece of code.
> 
> This is NOT worth it. Please someone ack the revert. This will
> hopefully stop the discussion and we can get back to daily business.
> 
> Volker
> 

Volker, the problem is that your patch didn't go far enough, I do not
write 'C', but I understood it. Andrews version is, in my opinion, over
the top, it really only needs to be something like this:

if ((persistent + non_persistent) != 1) {
		d_fprintf(stderr, "ERROR: you must specify either "
			  "--persistent or --non-persistent ");
		goto done;
	}

If you cannot work out from that you must specify 'persistent' OR
'non-persistent', then you shouldn't be writing code ;-)

Rowland



More information about the samba-technical mailing list