json: Redesign the callback to consume JSON values

The classical way to structure parser and lexer is to have the client
call the parser to get an abstract syntax tree, the parser call the
lexer to get the next token, and the lexer call some function to get
input characters.

Another way to structure them would be to have the client feed
characters to the lexer, the lexer feed tokens to the parser, and the
parser feed abstract syntax trees to some callback provided by the
client.  This way is more easily integrated into an event loop that
dispatches input characters as they arrive.

Our JSON parser is kind of between the two.  The lexer feeds tokens to
a "streamer" instead of a real parser.  The streamer accumulates
tokens until it got the sequence of tokens that comprise a single JSON
value (it counts curly braces and square brackets to decide).  It
feeds those token sequences to a callback provided by the client.  The
callback passes each token sequence to the parser, and gets back an
abstract syntax tree.

I figure it was done that way to make a straightforward recursive
descent parser possible.  "Get next token" becomes "pop the first
token off the token sequence".  Drawback: we need to store a complete
token sequence.  Each token eats 13 + input characters + malloc
overhead bytes.

Observations:

1. This is not the only way to use recursive descent.  If we replaced
   "get next token" by a coroutine yield, we could do without a
   streamer.

2. The lexer reports errors by passing a JSON_ERROR token to the
   streamer.  This communicates the offending input characters and
   their location, but no more.

3. The streamer reports errors by passing a null token sequence to the
   callback.  The (already poor) lexical error information is thrown
   away.

4. Having the callback receive a token sequence duplicates the code to
   convert token sequence to abstract syntax tree in every callback.

5. Known bug: the streamer silently drops incomplete token sequences.

This commit rectifies 4. by lifting the call of the parser from the
callbacks into the streamer.  Later commits will address 3. and 5.

The lifting removes a bug from qjson.c's parse_json(): it passed a
pointer to a non-null Error * in certain cases, as demonstrated by
check-qjson.c.

json_parser_parse() is now unused.  It's a stupid wrapper around
json_parser_parse_err().  Drop it, and rename json_parser_parse_err()
to json_parser_parse().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-35-armbru@redhat.com>
This commit is contained in:
Markus Armbruster 2018-08-23 18:40:01 +02:00
parent 037f244088
commit 62815d85ae
10 changed files with 39 additions and 54 deletions

View File

@ -16,7 +16,6 @@
#include "qemu-common.h" #include "qemu-common.h"
QObject *json_parser_parse(GQueue *tokens, va_list *ap); QObject *json_parser_parse(GQueue *tokens, va_list *ap, Error **errp);
QObject *json_parser_parse_err(GQueue *tokens, va_list *ap, Error **errp);
#endif #endif

View File

@ -25,7 +25,9 @@ typedef struct JSONToken {
typedef struct JSONMessageParser typedef struct JSONMessageParser
{ {
void (*emit)(struct JSONMessageParser *parser, GQueue *tokens); void (*emit)(void *opaque, QObject *json, Error *err);
void *opaque;
va_list *ap;
JSONLexer lexer; JSONLexer lexer;
int brace_count; int brace_count;
int bracket_count; int bracket_count;
@ -37,7 +39,9 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
JSONTokenType type, int x, int y); JSONTokenType type, int x, int y);
void json_message_parser_init(JSONMessageParser *parser, void json_message_parser_init(JSONMessageParser *parser,
void (*func)(JSONMessageParser *, GQueue *)); void (*emit)(void *opaque, QObject *json,
Error *err),
void *opaque, va_list *ap);
void json_message_parser_feed(JSONMessageParser *parser, void json_message_parser_feed(JSONMessageParser *parser,
const char *buffer, size_t size); const char *buffer, size_t size);

View File

@ -59,7 +59,6 @@
#include "qapi/qmp/qstring.h" #include "qapi/qmp/qstring.h"
#include "qapi/qmp/qjson.h" #include "qapi/qmp/qjson.h"
#include "qapi/qmp/json-streamer.h" #include "qapi/qmp/json-streamer.h"
#include "qapi/qmp/json-parser.h"
#include "qapi/qmp/qlist.h" #include "qapi/qmp/qlist.h"
#include "qom/object_interfaces.h" #include "qom/object_interfaces.h"
#include "trace-root.h" #include "trace-root.h"
@ -4256,18 +4255,15 @@ static void monitor_qmp_bh_dispatcher(void *data)
#define QMP_REQ_QUEUE_LEN_MAX (8) #define QMP_REQ_QUEUE_LEN_MAX (8)
static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens) static void handle_qmp_command(void *opaque, QObject *req, Error *err)
{ {
QObject *req, *id = NULL; Monitor *mon = opaque;
QObject *id = NULL;
QDict *qdict; QDict *qdict;
MonitorQMP *mon_qmp = container_of(parser, MonitorQMP, parser);
Monitor *mon = container_of(mon_qmp, Monitor, qmp);
Error *err = NULL;
QMPRequest *req_obj; QMPRequest *req_obj;
req = json_parser_parse_err(tokens, NULL, &err);
if (!req && !err) { if (!req && !err) {
/* json_parser_parse_err() sucks: can fail without setting @err */ /* json_parser_parse() sucks: can fail without setting @err */
error_setg(&err, QERR_JSON_PARSING); error_setg(&err, QERR_JSON_PARSING);
} }
@ -4465,7 +4461,8 @@ static void monitor_qmp_event(void *opaque, int event)
monitor_qmp_response_flush(mon); monitor_qmp_response_flush(mon);
monitor_qmp_cleanup_queues(mon); monitor_qmp_cleanup_queues(mon);
json_message_parser_destroy(&mon->qmp.parser); json_message_parser_destroy(&mon->qmp.parser);
json_message_parser_init(&mon->qmp.parser, handle_qmp_command); json_message_parser_init(&mon->qmp.parser, handle_qmp_command,
mon, NULL);
mon_refcount--; mon_refcount--;
monitor_fdsets_cleanup(); monitor_fdsets_cleanup();
break; break;
@ -4683,7 +4680,8 @@ void monitor_init(Chardev *chr, int flags)
if (monitor_is_qmp(mon)) { if (monitor_is_qmp(mon)) {
qemu_chr_fe_set_echo(&mon->chr, true); qemu_chr_fe_set_echo(&mon->chr, true);
json_message_parser_init(&mon->qmp.parser, handle_qmp_command); json_message_parser_init(&mon->qmp.parser, handle_qmp_command,
mon, NULL);
if (mon->use_io_thread) { if (mon->use_io_thread) {
/* /*
* Make sure the old iowatch is gone. It's possible when * Make sure the old iowatch is gone. It's possible when

View File

@ -14,7 +14,6 @@
#include "qemu/osdep.h" #include "qemu/osdep.h"
#include "qapi/error.h" #include "qapi/error.h"
#include "qapi/qmp/dispatch.h" #include "qapi/qmp/dispatch.h"
#include "qapi/qmp/json-parser.h"
#include "qapi/qmp/qdict.h" #include "qapi/qmp/qdict.h"
#include "qapi/qmp/qjson.h" #include "qapi/qmp/qjson.h"
#include "qapi/qmp/qbool.h" #include "qapi/qmp/qbool.h"

View File

@ -19,7 +19,6 @@
#include <sys/wait.h> #include <sys/wait.h>
#endif #endif
#include "qapi/qmp/json-streamer.h" #include "qapi/qmp/json-streamer.h"
#include "qapi/qmp/json-parser.h"
#include "qapi/qmp/qdict.h" #include "qapi/qmp/qdict.h"
#include "qapi/qmp/qjson.h" #include "qapi/qmp/qjson.h"
#include "qapi/qmp/qstring.h" #include "qapi/qmp/qstring.h"
@ -597,18 +596,13 @@ static void process_command(GAState *s, QDict *req)
} }
/* handle requests/control events coming in over the channel */ /* handle requests/control events coming in over the channel */
static void process_event(JSONMessageParser *parser, GQueue *tokens) static void process_event(void *opaque, QObject *obj, Error *err)
{ {
GAState *s = container_of(parser, GAState, parser); GAState *s = opaque;
QObject *obj;
QDict *req, *rsp; QDict *req, *rsp;
Error *err = NULL;
int ret; int ret;
g_assert(s && parser);
g_debug("process_event: called"); g_debug("process_event: called");
obj = json_parser_parse_err(tokens, NULL, &err);
if (err) { if (err) {
goto err; goto err;
} }
@ -1320,7 +1314,7 @@ static int run_agent(GAState *s, GAConfig *config, int socket_activation)
s->command_state = ga_command_state_new(); s->command_state = ga_command_state_new();
ga_command_state_init(s, s->command_state); ga_command_state_init(s, s->command_state);
ga_command_state_init_all(s->command_state); ga_command_state_init_all(s->command_state);
json_message_parser_init(&s->parser, process_event); json_message_parser_init(&s->parser, process_event, s, NULL);
#ifndef _WIN32 #ifndef _WIN32
if (!register_signal_handlers()) { if (!register_signal_handlers()) {

View File

@ -541,12 +541,7 @@ static QObject *parse_value(JSONParserContext *ctxt, va_list *ap)
} }
} }
QObject *json_parser_parse(GQueue *tokens, va_list *ap) QObject *json_parser_parse(GQueue *tokens, va_list *ap, Error **errp)
{
return json_parser_parse_err(tokens, ap, NULL);
}
QObject *json_parser_parse_err(GQueue *tokens, va_list *ap, Error **errp)
{ {
JSONParserContext ctxt = { .buf = tokens }; JSONParserContext ctxt = { .buf = tokens };
QObject *result; QObject *result;

View File

@ -14,6 +14,7 @@
#include "qemu/osdep.h" #include "qemu/osdep.h"
#include "qemu-common.h" #include "qemu-common.h"
#include "qapi/qmp/json-lexer.h" #include "qapi/qmp/json-lexer.h"
#include "qapi/qmp/json-parser.h"
#include "qapi/qmp/json-streamer.h" #include "qapi/qmp/json-streamer.h"
#define MAX_TOKEN_SIZE (64ULL << 20) #define MAX_TOKEN_SIZE (64ULL << 20)
@ -38,8 +39,9 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
JSONTokenType type, int x, int y) JSONTokenType type, int x, int y)
{ {
JSONMessageParser *parser = container_of(lexer, JSONMessageParser, lexer); JSONMessageParser *parser = container_of(lexer, JSONMessageParser, lexer);
Error *err = NULL;
JSONToken *token; JSONToken *token;
GQueue *tokens; QObject *json;
switch (type) { switch (type) {
case JSON_LCURLY: case JSON_LCURLY:
@ -97,19 +99,20 @@ out_emit:
/* send current list of tokens to parser and reset tokenizer */ /* send current list of tokens to parser and reset tokenizer */
parser->brace_count = 0; parser->brace_count = 0;
parser->bracket_count = 0; parser->bracket_count = 0;
/* parser->emit takes ownership of parser->tokens. Remove our own json = json_parser_parse(parser->tokens, parser->ap, &err);
* reference to parser->tokens before handing it out to parser->emit.
*/
tokens = parser->tokens;
parser->tokens = g_queue_new(); parser->tokens = g_queue_new();
parser->emit(parser, tokens);
parser->token_size = 0; parser->token_size = 0;
parser->emit(parser->opaque, json, err);
} }
void json_message_parser_init(JSONMessageParser *parser, void json_message_parser_init(JSONMessageParser *parser,
void (*func)(JSONMessageParser *, GQueue *)) void (*emit)(void *opaque, QObject *json,
Error *err),
void *opaque, va_list *ap)
{ {
parser->emit = func; parser->emit = emit;
parser->opaque = opaque;
parser->ap = ap;
parser->brace_count = 0; parser->brace_count = 0;
parser->bracket_count = 0; parser->bracket_count = 0;
parser->tokens = g_queue_new(); parser->tokens = g_queue_new();

View File

@ -13,8 +13,6 @@
#include "qemu/osdep.h" #include "qemu/osdep.h"
#include "qapi/error.h" #include "qapi/error.h"
#include "qapi/qmp/json-lexer.h"
#include "qapi/qmp/json-parser.h"
#include "qapi/qmp/json-streamer.h" #include "qapi/qmp/json-streamer.h"
#include "qapi/qmp/qjson.h" #include "qapi/qmp/qjson.h"
#include "qapi/qmp/qbool.h" #include "qapi/qmp/qbool.h"
@ -27,16 +25,16 @@
typedef struct JSONParsingState typedef struct JSONParsingState
{ {
JSONMessageParser parser; JSONMessageParser parser;
va_list *ap;
QObject *result; QObject *result;
Error *err; Error *err;
} JSONParsingState; } JSONParsingState;
static void parse_json(JSONMessageParser *parser, GQueue *tokens) static void consume_json(void *opaque, QObject *json, Error *err)
{ {
JSONParsingState *s = container_of(parser, JSONParsingState, parser); JSONParsingState *s = opaque;
s->result = json_parser_parse_err(tokens, s->ap, &s->err); s->result = json;
error_propagate(&s->err, err);
} }
/* /*
@ -54,9 +52,7 @@ static QObject *qobject_from_jsonv(const char *string, va_list *ap,
{ {
JSONParsingState state = {}; JSONParsingState state = {};
state.ap = ap; json_message_parser_init(&state.parser, consume_json, &state, ap);
json_message_parser_init(&state.parser, parse_json);
json_message_parser_feed(&state.parser, string, strlen(string)); json_message_parser_feed(&state.parser, string, strlen(string));
json_message_parser_flush(&state.parser); json_message_parser_flush(&state.parser);
json_message_parser_destroy(&state.parser); json_message_parser_destroy(&state.parser);

View File

@ -1438,7 +1438,6 @@ static void multiple_values(void)
qobject_unref(obj); qobject_unref(obj);
/* BUG simultaneously succeeds and fails */ /* BUG simultaneously succeeds and fails */
/* BUG calls json_parser_parse() with errp pointing to non-null */
obj = qobject_from_json("} true", &err); obj = qobject_from_json("} true", &err);
g_assert(qbool_get_bool(qobject_to(QBool, obj))); g_assert(qbool_get_bool(qobject_to(QBool, obj)));
error_free_or_abort(&err); error_free_or_abort(&err);

View File

@ -21,9 +21,9 @@
#include <sys/un.h> #include <sys/un.h>
#include "libqtest.h" #include "libqtest.h"
#include "qemu-common.h"
#include "qemu/cutils.h" #include "qemu/cutils.h"
#include "qapi/error.h" #include "qapi/error.h"
#include "qapi/qmp/json-parser.h"
#include "qapi/qmp/json-streamer.h" #include "qapi/qmp/json-streamer.h"
#include "qapi/qmp/qdict.h" #include "qapi/qmp/qdict.h"
#include "qapi/qmp/qjson.h" #include "qapi/qmp/qjson.h"
@ -446,12 +446,10 @@ typedef struct {
QDict *response; QDict *response;
} QMPResponseParser; } QMPResponseParser;
static void qmp_response(JSONMessageParser *parser, GQueue *tokens) static void qmp_response(void *opaque, QObject *obj, Error *err)
{ {
QMPResponseParser *qmp = container_of(parser, QMPResponseParser, parser); QMPResponseParser *qmp = opaque;
QObject *obj;
obj = json_parser_parse(tokens, NULL);
if (!obj) { if (!obj) {
fprintf(stderr, "QMP JSON response parsing failed\n"); fprintf(stderr, "QMP JSON response parsing failed\n");
abort(); abort();
@ -468,7 +466,7 @@ QDict *qmp_fd_receive(int fd)
bool log = getenv("QTEST_LOG") != NULL; bool log = getenv("QTEST_LOG") != NULL;
qmp.response = NULL; qmp.response = NULL;
json_message_parser_init(&qmp.parser, qmp_response); json_message_parser_init(&qmp.parser, qmp_response, &qmp, NULL);
while (!qmp.response) { while (!qmp.response) {
ssize_t len; ssize_t len;
char c; char c;