[PATCH] Re: [WIP] Log database changes.

Jeremy Allison jra at samba.org
Tue May 15 23:59:13 UTC 2018


On Wed, May 16, 2018 at 11:46:29AM +1200, Andrew Bartlett wrote:
> On Tue, 2018-05-15 at 15:58 -0700, Jeremy Allison wrote:
> > I do have one comment:
> > 
> > I really dislike the
> > 
> >  * Error handling:
> >  *
> >  * The json_object structure contains a boolean 'error'.  This is set whenever
> >  * an error is detected. All the library functions check this flag and return
> >  * immediately if it is set.
> >  *
> >  *	if (object->error) {
> >  *		return;
> >  *	}
> >  *
> >  * This allows the operations to be sequenced naturally with out the clutter
> >  * of error status checks.
> >  *
> > 
> > idiom. This was used in the original asn.1
> > codebase in Samba and it took me a *LONG* time to get
> > all the errors out of that.
> > 
> > Please don't consider error status checks as "clutter".
> > 
> > They're *not*. I'd much rather have correctly
> > error checked code than an automatic assumption
> > that errors will be handled later.
> 
> G'Day Jeremy.
> 
> Just to make sure I understand you.  As I'm sure you have seen, this
> patch just moves the existing helper functions (the work Gary did about
> a year ago) into a mini-library and places it in another folder, which
> is why it added the above documentation.

"I'm sure you have seen" - nope. As I'm sure you
know I haven't seen :-).

Just to confirm, I don't see this code currently in
the tree so it is new stuff being added, yeah ?

I just *hate* that idiom for new API's, don't think
we should expand it.

> Are you asking that I not merge this series, or can I merge this much
> and then you work with Gary to improve the API it as part of his
> ongoing work stream?

No, I don't want to block things - so you can merge
if you need to. I would like to work with Gary to
get this fixed though, I don't want it left like
this long term.

Cheers,

	Jeremy.



More information about the samba-technical mailing list