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

Jeremy Allison jra at samba.org
Tue May 15 22:58:43 UTC 2018


On Wed, May 16, 2018 at 10:53:08AM +1200, Andrew Bartlett via samba-technical wrote:
> On Fri, 2018-05-11 at 06:06 +1200, Andrew Bartlett via samba-technical
> wrote:
> > On Mon, 2018-05-07 at 18:05 +0200, Stefan Metzmacher via samba-
> > technical wrote:
> > > Hi Gary,
> > > 
> > > > Current state of this task.
> > > > 
> > > > Comments appreciated.
> > > 
> > > Most of the preparation like the session guid looks good.
> > 
> > I've reviewed and pushed the preparation patches patches that I could
> > get past an autobuild on GitLab CI.  The new common audit/json code
> > needs a talloc dep (fails to find talloc.h, a common trap for new
> > isolated subsystems), and I'll fix that next and push some more of the
> > prep code later.
> 
> Attached is the larger reviewed set of patches that I'm planning to put
> into the next autobuild.

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.

Jeremy.



More information about the samba-technical mailing list