Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'

[Eric Blake]

On 11/22/2016 04:06 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> On 11/21/2016 02:36 PM, Eric Blake wrote:
>>> The source of your problem is that your platform defines PRId64 as 'qd',
>>> but the qemu JSON parser only recognizes lld (POSIX) or I64d (mingw) for
>>> parsing 64-bit numbers.  We could enhance the JSON parser to recognize
>>> the non-standard qd in addition to the hack we already have for mingw,
> 
> Yes...
> 
>>> but I'd argue that using qobject_from_jsonf() is already less-than-useful.
>>
>> In fact, we are down to only a handful of users of our modified 'jsonf'
>> format (that is, strings that mix JSON with % modifiers):
>>
>> hw/pci/pcie_aer.c: build a 5-element QDict
>> monitor.c: build a 1-element QDict which contains a 2-element QDict
>> qapi/qmp-dispatch.c: build a 2-element QDict
>> qapi/qmp-event.c: Build a 2-element QDict
>>
>> plus the testsuite (check-qjson.c).
> 
> How did you find them?

git grep qobject_from_jsonf

I forgot about qobject_from_jsonv(), which is a bit more pervasive in
the testsuite, but even there, I only found a few additional uses of %
(that time, found by adding an assert(!strchr(format, '%')), then seeing
where it triggered during 'make check').  I should have the cleanup
series later today.  Technically, avoiding PRId64 in qmp-event.c is
worth having in 2.8 (it's a bug fix for Mac OS); but the rest of the
series is 2.9 material.  Note that at least one of the testsuite
conversions I'm making also used PRId64 - is the original poster even
running 'make check', as at least check-qga would be evidence of the
JSON parser failing to understand Mac's %qd.

> 
>>>   It's hard to argue that the
>>> complexity of maintaining our pseudo-printf JSON parser for constructing
>>> a QObject with one line is worth the effort compared to the three or
>>> four lines to construct the same QObject by hand.
>>
>> I'm severely tempted to just rip out all of the poorly-underdocumented %
>> parsing from the JSON parser, as it will simplify our code, without much
>> pain in converting the four real users to just manually build up the
>> same objects.
> 
> I kind of like the %-escapes, because they provide a compact and legible
> way to build QObjects.  But with so little use, they're hardly earning
> their keep.

What drives me most insane about them is that they are NOT the same
escapes as printf(), so the compiler can't help us with type safety, and
things like PRId64 don't always do what we want.  And except for %p for
injecting nested objects, most of our escapes are just as easy to
perform with g_strdup_printf() (injecting a string, integer, or
boolean), or by manual creation of the QDict.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature