Re: [PATCH v3 3/7] json-streamer: reuse parser

[Markus Armbruster <armbru@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> The push parser can be reset, so reuse it when the json-streamer
> detects a completed toplevel object.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  include/qobject/json-parser.h |  2 +-
>  qobject/json-streamer.c       | 11 ++++-------
>  2 files changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/include/qobject/json-parser.h b/include/qobject/json-parser.h
> index 05346fa816b..4c3d89f751f 100644
> --- a/include/qobject/json-parser.h
> +++ b/include/qobject/json-parser.h
> @@ -29,8 +29,8 @@ typedef struct JSONParserContext {
>  typedef struct JSONMessageParser {
>      void (*emit)(void *opaque, QObject *json, Error *err);
>      void *opaque;
> -    va_list *ap;
>      JSONLexer lexer;
> +    JSONParserContext parser;
>      int brace_count;
>      int bracket_count;
>      GQueue tokens;
> diff --git a/qobject/json-streamer.c b/qobject/json-streamer.c
> index 6c93e6fd78d..f3dfdcaea12 100644
> --- a/qobject/json-streamer.c
> +++ b/qobject/json-streamer.c
> @@ -32,7 +32,6 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
>                                  JSONTokenType type, int x, int y)
>  {
>      JSONMessageParser *parser = container_of(lexer, JSONMessageParser, lexer);
> -    JSONParserContext ctxt;
>      QObject *json = NULL;
>      Error *err = NULL;
>      JSONToken *token;
> @@ -90,26 +89,23 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
>          return;
>      }
>  
> -    json_parser_init(&ctxt, parser->ap);
> -
>      /* Process all tokens in the queue */
>      while (!g_queue_is_empty(&parser->tokens)) {
>          token = g_queue_pop_head(&parser->tokens);
> -        json = json_parser_feed(&ctxt, token, &err);
> +        json = json_parser_feed(&parser->parser, token, &err);
>          g_free(token);
>          if (json || err) {
>              break;
>          }
>      }
>  
> -    json_parser_destroy(&ctxt);
> -
>  out_emit:
>      parser->brace_count = 0;
>      parser->bracket_count = 0;
>      json_message_free_tokens(parser);
>      parser->token_size = 0;
>      parser->emit(parser->opaque, json, err);
> +    json_parser_reset(&parser->parser);
>  }

This made me stop and think.

I figure the more obvious *delta* would be to json_parser_reset() right
where json_parser_destroy() was.

But putting it here makes the code more obviously correct, I think.

No objection.  A hint in the commit message could've saved me a minute
or two.

>  
>  void json_message_parser_init(JSONMessageParser *parser,
> @@ -119,12 +115,12 @@ void json_message_parser_init(JSONMessageParser *parser,
>  {
>      parser->emit = emit;
>      parser->opaque = opaque;
> -    parser->ap = ap;
>      parser->brace_count = 0;
>      parser->bracket_count = 0;
>      g_queue_init(&parser->tokens);
>      parser->token_size = 0;
>  
> +    json_parser_init(&parser->parser, ap);
>      json_lexer_init(&parser->lexer, !!ap);
>  }
>  
> @@ -144,4 +140,5 @@ void json_message_parser_destroy(JSONMessageParser *parser)
>  {
>      json_lexer_destroy(&parser->lexer);
>      json_message_free_tokens(parser);
> +    json_parser_destroy(&parser->parser);
>  }

Reviewed-by: Markus Armbruster <armbru@redhat.com>