[Qemu-devel] [PATCH 34/56] json: Don't pass null @tokens to json_parser_parse()

Markus Armbruster posted 56 patches 7 years, 2 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 34/56] json: Don't pass null @tokens to json_parser_parse()
Posted by Markus Armbruster 7 years, 2 months ago
json_parser_parse() normally returns the QObject on success.  Except
it returns null when its @tokens argument is null.

Its only caller json_message_process_token() passes null @tokens when
emitting a lexical error.  The call is a rather opaque way to say json
= NULL then.

Simplify matters by lifting the assignment to json out of the emit
path: initialize json to null, set it to the value of
json_parser_parse() when there's no lexical error.  Drop the special
case from json_parser_parse().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qobject/json-parser.c   |  4 ----
 qobject/json-streamer.c | 25 ++++++++++++-------------
 2 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index 0196d511c3..0e4ea564ab 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -541,10 +541,6 @@ QObject *json_parser_parse(GQueue *tokens, va_list *ap, Error **errp)
     JSONParserContext ctxt = { .buf = tokens };
     QObject *result;
 
-    if (!tokens) {
-        return NULL;
-    }
-
     result = parse_value(&ctxt, ap);
 
     error_propagate(errp, ctxt.err);
diff --git a/qobject/json-streamer.c b/qobject/json-streamer.c
index 7fd0ff8756..0c33186e8e 100644
--- a/qobject/json-streamer.c
+++ b/qobject/json-streamer.c
@@ -39,9 +39,9 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
                                 JSONTokenType type, int x, int y)
 {
     JSONMessageParser *parser = container_of(lexer, JSONMessageParser, lexer);
+    QObject *json = NULL;
     Error *err = NULL;
     JSONToken *token;
-    QObject *json;
 
     switch (type) {
     case JSON_LCURLY:
@@ -72,34 +72,33 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
     g_queue_push_tail(parser->tokens, token);
 
     if (type == JSON_ERROR) {
-        goto out_emit_bad;
-    } else if (parser->brace_count < 0 ||
+        goto out_emit;
+    }
+
+    if (parser->brace_count < 0 ||
         parser->bracket_count < 0 ||
         (parser->brace_count == 0 &&
          parser->bracket_count == 0)) {
+        json = json_parser_parse(parser->tokens, parser->ap, &err);
+        parser->tokens = NULL;
         goto out_emit;
-    } else if (parser->token_size > MAX_TOKEN_SIZE ||
+    }
+
+    if (parser->token_size > MAX_TOKEN_SIZE ||
                g_queue_get_length(parser->tokens) > MAX_TOKEN_COUNT ||
                parser->bracket_count + parser->brace_count > MAX_NESTING) {
         /* Security consideration, we limit total memory allocated per object
          * and the maximum recursion depth that a message can force.
          */
-        goto out_emit_bad;
+        goto out_emit;
     }
 
     return;
 
-out_emit_bad:
-    /*
-     * Clear out token list and tell the parser to emit an error
-     * indication by passing it a NULL list
-     */
-    json_message_free_tokens(parser);
 out_emit:
-    /* send current list of tokens to parser and reset tokenizer */
     parser->brace_count = 0;
     parser->bracket_count = 0;
-    json = json_parser_parse(parser->tokens, parser->ap, &err);
+    json_message_free_tokens(parser);
     parser->tokens = g_queue_new();
     parser->token_size = 0;
     parser->emit(parser->opaque, json, err);
-- 
2.17.1


Re: [Qemu-devel] [PATCH 34/56] json: Don't pass null @tokens to json_parser_parse()
Posted by Eric Blake 7 years, 2 months ago
On 08/08/2018 07:03 AM, Markus Armbruster wrote:
> json_parser_parse() normally returns the QObject on success.  Except
> it returns null when its @tokens argument is null.
> 
> Its only caller json_message_process_token() passes null @tokens when
> emitting a lexical error.  The call is a rather opaque way to say json
> = NULL then.
> 
> Simplify matters by lifting the assignment to json out of the emit
> path: initialize json to null, set it to the value of
> json_parser_parse() when there's no lexical error.  Drop the special
> case from json_parser_parse().
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   qobject/json-parser.c   |  4 ----
>   qobject/json-streamer.c | 25 ++++++++++++-------------
>   2 files changed, 12 insertions(+), 17 deletions(-)
> 

Shorter and simpler.

Reviewed-by: Eric Blake <eblake@redhat.com>

> +++ b/qobject/json-streamer.c
> @@ -39,9 +39,9 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
>                                   JSONTokenType type, int x, int y)
>   {
>       JSONMessageParser *parser = container_of(lexer, JSONMessageParser, lexer);
> +    QObject *json = NULL;
>       Error *err = NULL;
>       JSONToken *token;
> -    QObject *json;

Why the churn in position?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 34/56] json: Don't pass null @tokens to json_parser_parse()
Posted by Markus Armbruster 7 years, 2 months ago
Eric Blake <eblake@redhat.com> writes:

> On 08/08/2018 07:03 AM, Markus Armbruster wrote:
>> json_parser_parse() normally returns the QObject on success.  Except
>> it returns null when its @tokens argument is null.
>>
>> Its only caller json_message_process_token() passes null @tokens when
>> emitting a lexical error.  The call is a rather opaque way to say json
>> = NULL then.
>>
>> Simplify matters by lifting the assignment to json out of the emit
>> path: initialize json to null, set it to the value of
>> json_parser_parse() when there's no lexical error.  Drop the special
>> case from json_parser_parse().
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   qobject/json-parser.c   |  4 ----
>>   qobject/json-streamer.c | 25 ++++++++++++-------------
>>   2 files changed, 12 insertions(+), 17 deletions(-)
>>
>
> Shorter and simpler.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
>> +++ b/qobject/json-streamer.c
>> @@ -39,9 +39,9 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
>>                                   JSONTokenType type, int x, int y)
>>   {
>>       JSONMessageParser *parser = container_of(lexer, JSONMessageParser, lexer);
>> +    QObject *json = NULL;
>>       Error *err = NULL;
>>       JSONToken *token;
>> -    QObject *json;
>
> Why the churn in position?

I like to put declarations with initializers before declarations without
initializers.  ObMovieQuote: "it's a symbol of my individuality, and my
belief in personal freedom."