Re: [Qemu-devel] RFC: libyajl for JSON

[Markus Armbruster <armbru@redhat.com>]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Tue, 3 Nov 2015 14:53:59 +0100
> Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>> On 03/11/2015 14:46, Luiz Capitulino wrote:
>> >> > Can you explain why that would make sense? :)  (Especially since there
>> >> > is another extension---JSON5---that does exactly what we're doing, so it
>> >> > probably wasn't that stupid an idea).
>> > Let's be pragmatic. *If* this is the only issue stopping us from
>> > dropping our own parser in favor of something more widely used and
>> > *if* libvirt doesn't make use of the feature, it's something we
>> > should strongly consider.
>> 
>> I'm not sure what's so bad about our parser that makes it worthwhile to:
>
> It's not that it's bad. It's about the advantages of dropping hundreds of
> lines of NIH code and switching to something more widely used.

I wish we would've / could've avoided NIH back then, but I'm not sure
getting rid of this piece of NIH now is worthwhile.

json-{lexer,parser,streamer}.[ch] together have 949 SLOC.  I'm not
counting tests, because we'd most likely keep them anyway.  This is less
than 0.1% of the QEMU source code :)

Maintenance hasn't been costing us a fortune exactly: 40 commits in six
years.

I'm annoyed by its relative shoddiness, but the prospect of having to
fix it up isn't something that keeps me up at night.

>                                                                Also,
> any maintenance time we spend on libyajl will also be automatically
> enjoyed by libvirt which is excellent.

Excellent indeed *if* upstream is responsive.

> On the other hand, I don't want to push too hard for it because I do
> recognize that switching has a cost and I won't be able to help with
> that myself.
>
>> 1) uglify all tests and make them inconsistent with the QAPI schemas,
>> which also uses single-quoted strings
>
> This doesn't seem hard to fix, we could pre-process the test files,
> say in Python, to add the needed escaping.

The test files are actually C code... sure you want to mangle C code in
Python?

>> 2) waste time finding a replacement for % interpolation (the best
>> replacement here would be to rewrite the tests in Python IMHO, but
>> that's not a small ask)
>
> Is this only used by tests?

No.

>                             Can you give an example of this feature?

Yes:

    static QDict *build_qmp_error_dict(Error *err)
    {
        QObject *obj;

        obj = qobject_from_jsonf("{ 'error': { 'class': %s, 'desc': %s } }",
                                 ErrorClass_lookup[error_get_class(err)],
                                 error_get_pretty(err));

        return qobject_to_qdict(obj);
    }

Builds a QDict with a single key "error".  Its value is a QDict with key
"class", value ErrorClass_lookup[error_get_class(err)], and key "desc",
value error_get_pretty(err), where "value" really means the C string
quoted for JSON and wrapped in a QString.

As I wrote elsewhere in the thread, we could build this by hand.  Much
less readable, but that might be tolerable, as the feature isn't widely
used.  I'm not thrilled about it, though.  It's too easy to forget the
quoting.

To find more examples, try "git-grep _json[fv]".

>> Just let's remove the weird (to not say worse) usage of QDict/QList to
>> store tokens...

Any replacement effort will have to compete on merits with fixing up
what we have.