[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