[Qemu-devel] [PATCH v2 2/4] qjson: do not save/restore contexts

[Paolo Bonzini]

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. ಠ_ಠ

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;
 }
-- 
2.5.0