Patch Fix JSON API

[Jeremy Allison]

On Wed, Jul 25, 2018 at 08:57:31AM +1200, Andrew Bartlett wrote:
> On Tue, 2018-07-24 at 13:30 -0700, Jeremy Allison via samba-technical
> wrote:
> > On Wed, Jul 25, 2018 at 06:59:00AM +1200, Gary Lockyer wrote:
> > > Hopefully the final patch set, have added _WARN_UNUSED_RESULT_ to the
> > > function prototypes.
> > > 
> > > Could I please get a review on this.
> > 
> > Will do asap Gary - thanks !
> > 
> > Jeremy.
> 
> Everything except:
> 
> [PATCH 3/7] audit logging  tests: Only run JSON tests in ad dc
> 
> Reviewed-by: Andrew Bartlett <abartlet at samba.org>
> 
> (The 'none' environment is built in autobuild with the AD DC, and runs
> many other tests.  We don't want to actually start the DC for unit
> tests). 
> 
> Jeremy,
> 
> As you called for the change I'll look forward to your comments.

json_to_string() can return NULL, so in PATCH 1/7 we
need a == NULL check on the below (yeah I know the
json_is_invalid() check above might also catch it, but
still your test code does explicitly check json_to_string()
returning NULL so I feel justified :-).

  * Write the json object to the audit logs as a formatted string
@@ -136,8 +115,16 @@ void audit_log_json(const char* prefix,
                    int debug_class,
                    int debug_level)
 {
-       TALLOC_CTX *ctx = talloc_new(NULL);
-       char *s = json_to_string(ctx, message);
+       TALLOC_CTX *ctx = NULL;
+       char *s = NULL;
+
+       if (json_is_invalid(message)) {
+               DBG_ERR("Invalid JSON object, unable to log\n");
+               return;
+       }
+
+       ctx = talloc_new(NULL);
+       s = json_to_string(ctx, message);
        DEBUGC(debug_class, debug_level, ("JSON %s: %s\n", prefix, s));
        TALLOC_FREE(ctx);

Other than that, looks great ! RB+ from me. Thanks a *LOT*
for this cleanup (although I'd disagree with the error handling
merely being "aesthetically displeasing" comment, due to bitter
experience with similar ASN.1 code :-) :-).

I'll defer to Andrew on the PATCH 3/7 comment, but if you
can fix the minor nit I raised (or tell me why I'm wrong)
I'm good to go with this patchset !

Cheers,

	Jeremy.