From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57019) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a0vOX-0005I3-K4 for qemu-devel@nongnu.org; Mon, 23 Nov 2015 12:59:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a0vOS-0005Jz-Jg for qemu-devel@nongnu.org; Mon, 23 Nov 2015 12:59:15 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38978) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a0vOS-0005Jl-9t for qemu-devel@nongnu.org; Mon, 23 Nov 2015 12:59:12 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id EC313C0032ED for ; Mon, 23 Nov 2015 17:59:11 +0000 (UTC) References: <1448300659-23559-1-git-send-email-pbonzini@redhat.com> <1448300659-23559-3-git-send-email-pbonzini@redhat.com> From: Laszlo Ersek Message-ID: <565353ED.1090502@redhat.com> Date: Mon, 23 Nov 2015 18:59:09 +0100 MIME-Version: 1.0 In-Reply-To: <1448300659-23559-3-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 2/4] qjson: do not save/restore contexts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: armbru@redhat.com 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. >=20 > 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. =E0=B2=A0_=E0=B2=A0 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 >=20 > Signed-off-by: Paolo Bonzini > --- > qobject/json-parser.c | 59 ++++++++++++++++++-------------------------= -------- > 1 file changed, 21 insertions(+), 38 deletions(-) >=20 > 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(JSONPars= erContext *ctxt) > return token; > } > =20 > -static JSONParserContext parser_context_save(JSONParserContext *ctxt) > -{ > - JSONParserContext saved_ctxt =3D {0}; > - saved_ctxt.tokens.pos =3D ctxt->tokens.pos; > - saved_ctxt.tokens.count =3D ctxt->tokens.count; > - saved_ctxt.tokens.buf =3D ctxt->tokens.buf; > - return saved_ctxt; > -} > - > -static void parser_context_restore(JSONParserContext *ctxt, > - JSONParserContext saved_ctxt) > -{ > - ctxt->tokens.pos =3D saved_ctxt.tokens.pos; > - ctxt->tokens.count =3D saved_ctxt.tokens.count; > - ctxt->tokens.buf =3D saved_ctxt.tokens.buf; > -} > - > static void tokens_append_from_iter(QObject *obj, void *opaque) > { > JSONParserContext *ctxt =3D opaque; > @@ -364,7 +347,6 @@ static void parser_context_free(JSONParserContext *= ctxt) > static int parse_pair(JSONParserContext *ctxt, QDict *dict, va_list *a= p) > { > QObject *key =3D NULL, *token =3D NULL, *value, *peek; > - JSONParserContext saved_ctxt =3D parser_context_save(ctxt); > =20 > peek =3D parser_context_peek_token(ctxt); > if (peek =3D=3D NULL) { > @@ -402,7 +384,6 @@ static int parse_pair(JSONParserContext *ctxt, QDic= t *dict, va_list *ap) > return 0; > =20 > out: > - parser_context_restore(ctxt, saved_ctxt); > qobject_decref(key); > =20 > return -1; > @@ -412,9 +393,8 @@ static QObject *parse_object(JSONParserContext *ctx= t, va_list *ap) > { > QDict *dict =3D NULL; > QObject *token, *peek; > - JSONParserContext saved_ctxt =3D parser_context_save(ctxt); > =20 > - token =3D parser_context_pop_token(ctxt); > + token =3D parser_context_peek_token(ctxt); > if (token =3D=3D NULL) { > goto out; > } > @@ -425,6 +405,7 @@ static QObject *parse_object(JSONParserContext *ctx= t, va_list *ap) > =20 > dict =3D qdict_new(); > =20 > + parser_context_pop_token(ctxt); > peek =3D parser_context_peek_token(ctxt); > if (peek =3D=3D NULL) { > parse_error(ctxt, NULL, "premature EOI"); > @@ -465,7 +446,6 @@ static QObject *parse_object(JSONParserContext *ctx= t, va_list *ap) > return QOBJECT(dict); > =20 > 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 =3D NULL; > QObject *token, *peek; > - JSONParserContext saved_ctxt =3D parser_context_save(ctxt); > =20 > - token =3D parser_context_pop_token(ctxt); > + token =3D parser_context_peek_token(ctxt); > if (token =3D=3D NULL) { > goto out; > } > @@ -487,6 +466,7 @@ static QObject *parse_array(JSONParserContext *ctxt= , va_list *ap) > =20 > list =3D qlist_new(); > =20 > + parser_context_pop_token(ctxt); > peek =3D parser_context_peek_token(ctxt); > if (peek =3D=3D NULL) { > parse_error(ctxt, NULL, "premature EOI"); > @@ -537,7 +517,6 @@ static QObject *parse_array(JSONParserContext *ctxt= , va_list *ap) > return QOBJECT(list); > =20 > 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 =3D parser_context_save(ctxt); > =20 > - token =3D parser_context_pop_token(ctxt); > + token =3D parser_context_peek_token(ctxt); > if (token =3D=3D NULL) { > goto out; > } > @@ -556,6 +534,7 @@ static QObject *parse_keyword(JSONParserContext *ct= xt) > goto out; > } > =20 > + parser_context_pop_token(ctxt); > if (token_is_keyword(token, "true")) { > ret =3D QOBJECT(qbool_from_bool(true)); > } else if (token_is_keyword(token, "false")) { > @@ -570,7 +549,6 @@ static QObject *parse_keyword(JSONParserContext *ct= xt) > return ret; > =20 > out:=20 > - parser_context_restore(ctxt, saved_ctxt); > =20 > return NULL; > } > @@ -578,17 +556,21 @@ out: > static QObject *parse_escape(JSONParserContext *ctxt, va_list *ap) > { > QObject *token =3D NULL, *obj; > - JSONParserContext saved_ctxt =3D parser_context_save(ctxt); > =20 > if (ap =3D=3D NULL) { > goto out; > } > =20 > - token =3D parser_context_pop_token(ctxt); > + token =3D parser_context_peek_token(ctxt); > if (token =3D=3D NULL) { > goto out; > } > =20 > + if (token_get_type(token) !=3D JSON_ESCAPE) { > + goto out; > + } > + > + parser_context_pop_token(ctxt); > if (token_is_escape(token, "%p")) { > obj =3D va_arg(*ap, QObject *); > } else if (token_is_escape(token, "%i")) { > @@ -611,7 +593,6 @@ static QObject *parse_escape(JSONParserContext *ctx= t, va_list *ap) > return obj; > =20 > out: > - parser_context_restore(ctxt, saved_ctxt); > =20 > return NULL; > } > @@ -619,15 +600,15 @@ out: > static QObject *parse_literal(JSONParserContext *ctxt) > { > QObject *token, *obj; > - JSONParserContext saved_ctxt =3D parser_context_save(ctxt); > =20 > - token =3D parser_context_pop_token(ctxt); > + token =3D parser_context_peek_token(ctxt); > if (token =3D=3D NULL) { > goto out; > } > =20 > switch (token_get_type(token)) { > case JSON_STRING: > + parser_context_pop_token(ctxt); > obj =3D QOBJECT(qstring_from_escaped_str(ctxt, token)); > break; > case JSON_INTEGER: { > @@ -645,15 +626,18 @@ static QObject *parse_literal(JSONParserContext *= ctxt) > */ > int64_t value; > =20 > + parser_context_pop_token(ctxt); > errno =3D 0; /* strtoll doesn't set errno on success */ > value =3D strtoll(token_get_value(token), NULL, 10); > if (errno !=3D ERANGE) { > obj =3D 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 =3D QOBJECT(qfloat_from_double(strtod(token_get_value(toke= n), NULL))); > break; > @@ -664,7 +648,6 @@ static QObject *parse_literal(JSONParserContext *ct= xt) > return obj; > =20 > out: > - parser_context_restore(ctxt, saved_ctxt); > =20 > return NULL; > } > @@ -681,11 +664,11 @@ static QObject *parse_value(JSONParserContext *ct= xt, va_list *ap) > obj =3D parse_escape(ctxt, ap); > } > if (obj =3D=3D NULL) { > - obj =3D parse_keyword(ctxt); > - }=20 > - if (obj =3D=3D NULL) { > obj =3D parse_literal(ctxt); > } > + if (obj =3D=3D NULL) { > + obj =3D parse_keyword(ctxt); > + } > =20 > return obj; > } >=20