[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/4] qjson: do not save/restore contexts
From: |
Laszlo Ersek |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/4] qjson: do not save/restore contexts |
Date: |
Mon, 23 Nov 2015 18:59:09 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 11/23/15 18:44, Paolo Bonzini wrote:
> JSON is LL(1) and our parser indeed needs only 1 token lookahead.
> Saving the parser context is mostly unnecessary; we can replace it
> with peeking at the next token, or remove it altogether when the
> restore only happens on errors. The token list is destroyed anyway
> on errors.
>
> The only interesting thing is that parse_keyword always eats
> a TOKEN_KEYWORD, even if it is invalid, so it must come last in
> parse_value (otherwise, NULL is returned, parse_literal is invoked
> and it tries to peek beyond end of input). This is caught by
> /errors/unterminated/literal, which actually checks for an unterminated
> keyword. ಠ_ಠ
Is it accepted practice to put UTF-8 in commit messages? (Or, actually,
anywhere in patches, except maybe the notes section?)
I'd recommend o_O.
Thanks
Laszlo
>
> Signed-off-by: Paolo Bonzini <address@hidden>
> ---
> qobject/json-parser.c | 59
> ++++++++++++++++++---------------------------------
> 1 file changed, 21 insertions(+), 38 deletions(-)
>
> diff --git a/qobject/json-parser.c b/qobject/json-parser.c
> index ac991ba..7a287ea 100644
> --- a/qobject/json-parser.c
> +++ b/qobject/json-parser.c
> @@ -296,23 +296,6 @@ static QObject
> *parser_context_peek_token(JSONParserContext *ctxt)
> return token;
> }
>
> -static JSONParserContext parser_context_save(JSONParserContext *ctxt)
> -{
> - JSONParserContext saved_ctxt = {0};
> - saved_ctxt.tokens.pos = ctxt->tokens.pos;
> - saved_ctxt.tokens.count = ctxt->tokens.count;
> - saved_ctxt.tokens.buf = ctxt->tokens.buf;
> - return saved_ctxt;
> -}
> -
> -static void parser_context_restore(JSONParserContext *ctxt,
> - JSONParserContext saved_ctxt)
> -{
> - ctxt->tokens.pos = saved_ctxt.tokens.pos;
> - ctxt->tokens.count = saved_ctxt.tokens.count;
> - ctxt->tokens.buf = saved_ctxt.tokens.buf;
> -}
> -
> static void tokens_append_from_iter(QObject *obj, void *opaque)
> {
> JSONParserContext *ctxt = opaque;
> @@ -364,7 +347,6 @@ static void parser_context_free(JSONParserContext *ctxt)
> static int parse_pair(JSONParserContext *ctxt, QDict *dict, va_list *ap)
> {
> QObject *key = NULL, *token = NULL, *value, *peek;
> - JSONParserContext saved_ctxt = parser_context_save(ctxt);
>
> peek = parser_context_peek_token(ctxt);
> if (peek == NULL) {
> @@ -402,7 +384,6 @@ static int parse_pair(JSONParserContext *ctxt, QDict
> *dict, va_list *ap)
> return 0;
>
> out:
> - parser_context_restore(ctxt, saved_ctxt);
> qobject_decref(key);
>
> return -1;
> @@ -412,9 +393,8 @@ static QObject *parse_object(JSONParserContext *ctxt,
> va_list *ap)
> {
> QDict *dict = NULL;
> QObject *token, *peek;
> - JSONParserContext saved_ctxt = parser_context_save(ctxt);
>
> - token = parser_context_pop_token(ctxt);
> + token = parser_context_peek_token(ctxt);
> if (token == NULL) {
> goto out;
> }
> @@ -425,6 +405,7 @@ static QObject *parse_object(JSONParserContext *ctxt,
> va_list *ap)
>
> dict = qdict_new();
>
> + parser_context_pop_token(ctxt);
> peek = parser_context_peek_token(ctxt);
> if (peek == NULL) {
> parse_error(ctxt, NULL, "premature EOI");
> @@ -465,7 +446,6 @@ static QObject *parse_object(JSONParserContext *ctxt,
> va_list *ap)
> return QOBJECT(dict);
>
> out:
> - parser_context_restore(ctxt, saved_ctxt);
> QDECREF(dict);
> return NULL;
> }
> @@ -474,9 +454,8 @@ static QObject *parse_array(JSONParserContext *ctxt,
> va_list *ap)
> {
> QList *list = NULL;
> QObject *token, *peek;
> - JSONParserContext saved_ctxt = parser_context_save(ctxt);
>
> - token = parser_context_pop_token(ctxt);
> + token = parser_context_peek_token(ctxt);
> if (token == NULL) {
> goto out;
> }
> @@ -487,6 +466,7 @@ static QObject *parse_array(JSONParserContext *ctxt,
> va_list *ap)
>
> list = qlist_new();
>
> + parser_context_pop_token(ctxt);
> peek = parser_context_peek_token(ctxt);
> if (peek == NULL) {
> parse_error(ctxt, NULL, "premature EOI");
> @@ -537,7 +517,6 @@ static QObject *parse_array(JSONParserContext *ctxt,
> va_list *ap)
> return QOBJECT(list);
>
> out:
> - parser_context_restore(ctxt, saved_ctxt);
> QDECREF(list);
> return NULL;
> }
> @@ -545,9 +524,8 @@ out:
> static QObject *parse_keyword(JSONParserContext *ctxt)
> {
> QObject *token, *ret;
> - JSONParserContext saved_ctxt = parser_context_save(ctxt);
>
> - token = parser_context_pop_token(ctxt);
> + token = parser_context_peek_token(ctxt);
> if (token == NULL) {
> goto out;
> }
> @@ -556,6 +534,7 @@ static QObject *parse_keyword(JSONParserContext *ctxt)
> goto out;
> }
>
> + parser_context_pop_token(ctxt);
> if (token_is_keyword(token, "true")) {
> ret = QOBJECT(qbool_from_bool(true));
> } else if (token_is_keyword(token, "false")) {
> @@ -570,7 +549,6 @@ static QObject *parse_keyword(JSONParserContext *ctxt)
> return ret;
>
> out:
> - parser_context_restore(ctxt, saved_ctxt);
>
> return NULL;
> }
> @@ -578,17 +556,21 @@ out:
> static QObject *parse_escape(JSONParserContext *ctxt, va_list *ap)
> {
> QObject *token = NULL, *obj;
> - JSONParserContext saved_ctxt = parser_context_save(ctxt);
>
> if (ap == NULL) {
> goto out;
> }
>
> - token = parser_context_pop_token(ctxt);
> + token = parser_context_peek_token(ctxt);
> if (token == NULL) {
> goto out;
> }
>
> + if (token_get_type(token) != JSON_ESCAPE) {
> + goto out;
> + }
> +
> + parser_context_pop_token(ctxt);
> if (token_is_escape(token, "%p")) {
> obj = va_arg(*ap, QObject *);
> } else if (token_is_escape(token, "%i")) {
> @@ -611,7 +593,6 @@ static QObject *parse_escape(JSONParserContext *ctxt,
> va_list *ap)
> return obj;
>
> out:
> - parser_context_restore(ctxt, saved_ctxt);
>
> return NULL;
> }
> @@ -619,15 +600,15 @@ out:
> static QObject *parse_literal(JSONParserContext *ctxt)
> {
> QObject *token, *obj;
> - JSONParserContext saved_ctxt = parser_context_save(ctxt);
>
> - token = parser_context_pop_token(ctxt);
> + token = parser_context_peek_token(ctxt);
> if (token == NULL) {
> goto out;
> }
>
> switch (token_get_type(token)) {
> case JSON_STRING:
> + parser_context_pop_token(ctxt);
> obj = QOBJECT(qstring_from_escaped_str(ctxt, token));
> break;
> case JSON_INTEGER: {
> @@ -645,15 +626,18 @@ static QObject *parse_literal(JSONParserContext *ctxt)
> */
> int64_t value;
>
> + parser_context_pop_token(ctxt);
> errno = 0; /* strtoll doesn't set errno on success */
> value = strtoll(token_get_value(token), NULL, 10);
> if (errno != ERANGE) {
> obj = QOBJECT(qint_from_int(value));
> break;
> }
> - /* fall through to JSON_FLOAT */
> + goto parse_float;
> }
> case JSON_FLOAT:
> + parser_context_pop_token(ctxt);
> + parse_float:
> /* FIXME dependent on locale */
> obj = QOBJECT(qfloat_from_double(strtod(token_get_value(token),
> NULL)));
> break;
> @@ -664,7 +648,6 @@ static QObject *parse_literal(JSONParserContext *ctxt)
> return obj;
>
> out:
> - parser_context_restore(ctxt, saved_ctxt);
>
> return NULL;
> }
> @@ -681,11 +664,11 @@ static QObject *parse_value(JSONParserContext *ctxt,
> va_list *ap)
> obj = parse_escape(ctxt, ap);
> }
> if (obj == NULL) {
> - obj = parse_keyword(ctxt);
> - }
> - if (obj == NULL) {
> obj = parse_literal(ctxt);
> }
> + if (obj == NULL) {
> + obj = parse_keyword(ctxt);
> + }
>
> return obj;
> }
>
- [Qemu-devel] [PATCH v2 for-2.5? 0/4] qjson: save a lot of memory, Paolo Bonzini, 2015/11/23
- [Qemu-devel] [PATCH v2 1/4] qjson: replace QString in JSONLexer with GString, Paolo Bonzini, 2015/11/23
- [Qemu-devel] [PATCH v2 2/4] qjson: do not save/restore contexts, Paolo Bonzini, 2015/11/23
- Re: [Qemu-devel] [PATCH v2 2/4] qjson: do not save/restore contexts, Gerd Hoffmann, 2015/11/24
- Re: [Qemu-devel] [PATCH v2 2/4] qjson: do not save/restore contexts, Laszlo Ersek, 2015/11/24
- Re: [Qemu-devel] [PATCH v2 2/4] qjson: do not save/restore contexts, Paolo Bonzini, 2015/11/24
- Re: [Qemu-devel] [PATCH v2 2/4] qjson: do not save/restore contexts, Fam Zheng, 2015/11/24
- Re: [Qemu-devel] [PATCH v2 2/4] qjson: do not save/restore contexts, Paolo Bonzini, 2015/11/24
- Re: [Qemu-devel] [PATCH v2 2/4] qjson: do not save/restore contexts, Markus Armbruster, 2015/11/24