All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Luiz Capitulino <lcapitulino@redhat.com>
Subject: Re: [Qemu-devel] RFC: libyajl for JSON
Date: Mon, 02 Nov 2015 13:56:41 +0100	[thread overview]
Message-ID: <87lhagwody.fsf@blackfin.pond.sub.org> (raw)
In-Reply-To: <56374DC5.2050808@redhat.com> (Paolo Bonzini's message of "Mon, 2 Nov 2015 12:49:25 +0100")

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 02/11/2015 09:40, Markus Armbruster wrote:
>> 
>> * Optional interpolation
>> 
>>   If you pass a va_list to the parser, it replaces certain escape
>>   sequences by values from that va_list.  The escape sequences are a
>>   subset of printf conversion specifiers, to enable compile-time
>>   checking with __attribute__((format...)).
>> 
>>   We used to rely on this for creating "rich" error objects, but those
>>   are long gone (commit df1e608).  Perhaps two dozen uses are left.
>
> Most of these are probably in the tests, where they are used to send QMP
> commands.

Yes.  Not all of them, though.

>>   We could convert them to use "normal" interpolation, i.e. first format
>>   the arguments as JSON, then interpolate with g_strdup_printf() or
>>   similar, then parse.  Formatting only to parse it right back is
>>   inelegant.  Also inefficient, but that probably doesn't matter here.
>
> I don't care about inefficiency or elegance, but I care about bugs.  One
> difference between printf-ing \"%s\" and JSON-parsing %s is that the
> latter performs backslash escaping.  The lack of backslash escaping has
> been the source of bugs in the past, so it's important that the API
> remains as convenient as it is now.

That's why I wrote "first format as JSON, then interpolate".  Formatting
with printf() does not work precisely because it doesn't format as JSON.
We'd have to use our JSON formatter.  Any bugs it may have we need to
fix anyway, unless we also replace it wholesale.

> There is one alternative: rewriting tests in Python.  I'm only
> half-kidding, probably less than half.  It would be great for example to
> reuse the work on BITS (http://biosbits.org/) within qtest.
>
[On representing JSON numbers as int64_t when they fit:]
>>   Unlike the extensions above, this one is essential: any parser that
>>   can't do it is useless for us.  Can yajl do it?
>
> I think it does.

Good.

One more thing on our JSON parser.

A classical parser gets passed a source of characters (string, file
descriptor, whatever), parses until it reaches a terminating state, then
returns an abstract syntax tree (AST).  Basically a function mapping a
character source to an AST.

Our JSON parser has inverted control flow: it gets fed characters on at
a time, by the main loop via chardev IOReadHandler, and when it reaches
a terminating state, it passes the parse result to a callback.

Except this is an oversimplification.  We actually have two parsers, a
stupid one that can only count braces and brackets (json-streamer.c),
and the real one (json-parser.c).  The stupid one passes a list of
tokens to the callback, which runs the real parser to parse the tokens
into a QObject (this is our AST), then does whatever needs to be done
with the QObject.

But this is detail, the point remains that the current JSON parsing
machinery gets fed characters and the code consuming the parse lives in
a callback, and any replacement also needs to be fit into the main loop
somehow.

  reply	other threads:[~2015-11-02 12:56 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-30 19:45 [Qemu-devel] RFC: libyajl for JSON Eric Blake
2015-10-30 20:16 ` Peter Maydell
2015-11-02  8:40 ` Markus Armbruster
2015-11-02 11:49   ` Paolo Bonzini
2015-11-02 12:56     ` Markus Armbruster [this message]
2015-11-02 13:47       ` Paolo Bonzini
2015-11-02 14:10   ` Stefan Hajnoczi
2015-11-02 15:46   ` Eric Blake
2015-11-02 19:10     ` Markus Armbruster
2015-11-02 20:08       ` Eric Blake
2015-11-03  7:17         ` Markus Armbruster
2015-11-03 13:19           ` Luiz Capitulino
2015-11-03 13:38             ` Paolo Bonzini
2015-11-03 13:46               ` Luiz Capitulino
2015-11-03 13:53                 ` Paolo Bonzini
2015-11-03 14:26                   ` Luiz Capitulino
2015-11-03 14:53                     ` Paolo Bonzini
2015-11-03 15:08                     ` Markus Armbruster
2015-11-03 13:40             ` Eric Blake
2015-11-03 13:44               ` Daniel P. Berrange
2015-11-03 13:53               ` Luiz Capitulino
2015-11-02 13:17 ` Luiz Capitulino

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=87lhagwody.fsf@blackfin.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=lcapitulino@redhat.com \
    --cc=pbonzini@redhat.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.