Re: [Qemu-devel] [PATCH 1/4] qjson: Improve debugging

[Luiz Capitulino <lcapitulino@redhat.com>]

On Fri, 05 Feb 2010 10:13:33 +0100
Markus Armbruster <armbru@redhat.com> wrote:

> Anthony Liguori <anthony@codemonkey.ws> writes:
> 
> > On 02/04/2010 02:13 PM, Luiz Capitulino wrote:
> >> Add an assert() to qobject_from_jsonf() to assure that the returned
> >> QObject is not NULL. Currently this is duplicated in the callers.
> >>
> >> Signed-off-by: Luiz Capitulino<lcapitulino@redhat.com>
> >> ---
> >>   qjson.c |    1 +
> >>   1 files changed, 1 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/qjson.c b/qjson.c
> >> index 9ad8a91..0922c06 100644
> >> --- a/qjson.c
> >> +++ b/qjson.c
> >> @@ -62,6 +62,7 @@ QObject *qobject_from_jsonf(const char *string, ...)
> >>       obj = qobject_from_jsonv(string,&ap);
> >>       va_end(ap);
> >>
> >> +    assert(obj != NULL);
> >>    
> >
> > This is wrong.  We may get JSON from an untrusted source.  Callers
> > need to deal with failure appropriately.
> >
> > It just so happens that we only parse JSON from an untrusted source
> > via qobject_from_json(), but the trust relationship is not obvious
> > given the two functions in their current form.
> 
> We have many uses of qobject_from_jsonf() with a literal argument, and
> more to come.  Making them all deal with failure would be tedious and
> clutter the code.  What about a wrapper function that cannot fail?

 As far as I can understand, qobject_from_jsonf() is supposed to
be that wrapper already.