Re: [PATCH 4/5] json-streamer: do not heap-allocate JSONToken

[Markus Armbruster <armbru@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> This is not needed with a push parser.  Since it processes tokens
> immediately, the JSONToken can be created directly on the stack
> and does not need to copy the lexer's string data.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  qobject/json-parser-int.h |  8 ++++++--
>  qobject/json-parser.c     | 18 ------------------
>  qobject/json-streamer.c   |  4 ++--
>  3 files changed, 8 insertions(+), 22 deletions(-)
>
> diff --git a/qobject/json-parser-int.h b/qobject/json-parser-int.h
> index 1f435cb8eb2..5a6b5c9af90 100644
> --- a/qobject/json-parser-int.h
> +++ b/qobject/json-parser-int.h
> @@ -35,7 +35,12 @@ typedef enum json_token_type {
>      JSON_MAX = JSON_END_OF_INPUT
>  } JSONTokenType;
>  
> -typedef struct JSONToken JSONToken;
> +typedef struct JSONToken {
> +    JSONTokenType type;
> +    int x;
> +    int y;
> +    char *str;
> +} JSONToken;
>  
>  /* json-lexer.c */
>  void json_lexer_init(JSONLexer *lexer, bool enable_interpolation);
> @@ -48,7 +53,6 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
>                                  JSONTokenType type, int x, int y);
>  
>  /* json-parser.c */
> -JSONToken *json_token(JSONTokenType type, int x, int y, GString *tokstr);
>  void json_parser_init(JSONParserContext *ctxt, va_list *ap);
>  void json_parser_reset(JSONParserContext *ctxt);
>  QObject *json_parser_feed(JSONParserContext *ctxt, const JSONToken *token, Error **errp);
> diff --git a/qobject/json-parser.c b/qobject/json-parser.c
> index 7abdea4dacb..2fede59842f 100644
> --- a/qobject/json-parser.c
> +++ b/qobject/json-parser.c
> @@ -24,13 +24,6 @@
>  #include "qobject/qstring.h"
>  #include "json-parser-int.h"
>  
> -struct JSONToken {
> -    JSONTokenType type;
> -    int x;
> -    int y;
> -    char str[];
> -};
> -
>  /*
>   * Objects: { } | { members }
>   * - Empty: { -> AFTER_LCURLY -> }
> @@ -529,17 +522,6 @@ static QObject *json_parser_parse_token(JSONParserContext *ctxt, const JSONToken
>      return NULL;
>  }
>  
> -JSONToken *json_token(JSONTokenType type, int x, int y, GString *tokstr)
> -{
> -    JSONToken *token = g_malloc(sizeof(JSONToken) + tokstr->len + 1);
> -
> -    token->type = type;
> -    memcpy(token->str, tokstr->str, tokstr->len);
> -    token->str[tokstr->len] = 0;
> -    token->x = x;
> -    token->y = y;
> -    return token;
> -}
>  
>  void json_parser_reset(JSONParserContext *ctxt)
>  {
> diff --git a/qobject/json-streamer.c b/qobject/json-streamer.c
> index a1210128ac1..07e0ef51ce3 100644
> --- a/qobject/json-streamer.c
> +++ b/qobject/json-streamer.c
> @@ -23,7 +23,7 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
>                                  JSONTokenType type, int x, int y)
>  {
>      JSONMessageParser *parser = container_of(lexer, JSONMessageParser, lexer);
> -    g_autofree JSONToken *token = json_token(type, x, y, input);
> +    JSONToken token = (JSONToken) { .type = type, .x = x, .y = y, .str = input->str };

checkpatch points out:

    0004-json-streamer-do-not-heap-allocate-JSONToken.patch:89: WARNING: line over 80 characters

Easy enough to avoid.

>      Error *err = NULL;
>  
>      parser->token_size += input->len;
> @@ -64,7 +64,7 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
>          } else if (parser->bracket_count + parser->brace_count > MAX_NESTING) {
>              error_setg(&err, "JSON nesting depth limit exceeded");
>          } else {
> -            QObject *json = json_parser_feed(&parser->parser, token, &err);
> +            QObject *json = json_parser_feed(&parser->parser, &token, &err);
>              if (json) {
>                  parser->emit(parser->opaque, json, NULL);
>              }

With the long line tidied up
Reviewed-by: Markus Armbruster <armbru@redhat.com>