From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: G 3 <programmingkidx@gmail.com>,
Paolo Bonzini <pbonzini@redhat.com>,
qemu-devel qemu-devel <qemu-devel@nongnu.org>
Subject: Re: [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL'
Date: Tue, 22 Nov 2016 06:41:33 -0600 [thread overview]
Message-ID: <fc46154c-5558-9606-2872-d4e294bcf38c@redhat.com> (raw)
In-Reply-To: <87twaz3k2h.fsf@dusky.pond.sub.org>
[-- Attachment #1: Type: text/plain, Size: 2867 bytes --]
On 11/22/2016 04:06 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> 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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
next prev parent reply other threads:[~2016-11-22 12:41 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-11-21 6:30 [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL' G 3
2016-11-21 9:13 ` Markus Armbruster
2016-11-21 10:02 ` Paolo Bonzini
2016-11-21 20:12 ` G 3
2016-11-21 20:36 ` Eric Blake
2016-11-21 20:46 ` Eric Blake
2016-11-21 20:53 ` G 3
2016-11-22 6:49 ` Paolo Bonzini
2016-11-22 10:06 ` Markus Armbruster
2016-11-22 12:41 ` Eric Blake [this message]
2016-11-22 12:54 ` Paolo Bonzini
2016-11-22 15:05 ` Markus Armbruster
2016-11-22 15:02 ` G 3
2016-11-22 15:13 ` Eric Blake
2016-11-22 16:00 ` [Qemu-devel] check-qjson failure G 3
2016-11-22 22:42 ` Eric Blake
2016-11-22 22:22 ` [Qemu-devel] qobject/qjson.c:69: failed assertion `obj != NULL' G 3
2016-11-22 22:41 ` Eric Blake
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fc46154c-5558-9606-2872-d4e294bcf38c@redhat.com \
--to=eblake@redhat.com \
--cc=armbru@redhat.com \
--cc=pbonzini@redhat.com \
--cc=programmingkidx@gmail.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).