Re: [Qemu-devel] [PATCH 6/6] json: Eliminate lexer state IN_WHITESPACE, pseudo-token JSON_SKIP

[Eric Blake]

On 08/27/2018 02:00 AM, Markus Armbruster wrote:
> The lexer ignores whitespace like this:
>
>           on whitespace      on non-ws   spontaneously
>      IN_START --> IN_WHITESPACE --> JSON_SKIP --> IN_START
>                      ^    |
>                       \__/  on whitespace
>
> This accumulates a whitespace token in state IN_WHITESPACE, only to
> throw it away on the transition via JSON_SKIP to the start state.
> Wasteful.  Go from IN_START to IN_START on whitspace directly,

s/whitspace/whitespace/

> dropping the whitespace character.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>   qobject/json-lexer.c      | 22 +++++-----------------
>   qobject/json-parser-int.h |  1 -
>   2 files changed, 5 insertions(+), 18 deletions(-)
>
> @@ -263,10 +253,10 @@ static const uint8_t json_lexer[][256] =  {
>           [','] = JSON_COMMA,
>           [':'] = JSON_COLON,
>           ['a' ... 'z'] = IN_KEYWORD,
> -        [' '] = IN_WHITESPACE,
> -        ['\t'] = IN_WHITESPACE,
> -        ['\r'] = IN_WHITESPACE,
> -        ['\n'] = IN_WHITESPACE,
> +        [' '] = IN_START,
> +        ['\t'] = IN_START,
> +        ['\r'] = IN_START,
> +        ['\n'] = IN_START,
>       },
>       [IN_START_INTERP]['%'] = IN_INTERP,

Don't you need to set [IN_START_INTERP][' '] to IN_START_INTERP, rather 
than IN_START?  Otherwise, the presence of skipped whitespace would 
change whether interpolation happens.  (At least, that's what you had in 
an earlier version of this patch).

>   };
> @@ -323,10 +313,8 @@ static void json_lexer_feed_char(JSONLexer *lexer, char 
> ch, bool flush)
>               json_message_process_token(lexer, lexer->token, new_state,
>                                          lexer->x, lexer->y);
>               /* fall through */
> -        case JSON_SKIP:
> -            g_string_truncate(lexer->token, 0);
> -            /* fall through */
>           case IN_START:
> +            g_string_truncate(lexer->token, 0);
>               new_state = lexer->start_state;

Oh, I see. We are magically reverting to the correct start state if the 
requested transition reports IN_START, rather than blindly using IN_START.

Reviewed-by: Eric Blake <address@hidden>

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