Paolo Bonzini <pbonz...@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.