Paolo Bonzini <[email protected]> writes:
> It makes no sense to let brace_count and bracket_count go negative,
> also because it immediately ends error recovery and sets them both
> back to zero. Instead set them to zero *before* choosing
> whether to process the token queue; this makes it possible to
> have the fields as unsigned.
>
> Note that JSON_END_OF_INPUT now forces the parentheses to appear
> balanced, so that the queue is emptied and an error is reported;
> hence, the "type != JSON_END_OF_INPUT" condition can be removed.
>
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
> include/qobject/json-parser.h | 4 ++--
> qobject/json-streamer.c | 19 ++++++++++++++++---
> 2 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/include/qobject/json-parser.h b/include/qobject/json-parser.h
> index 4c3d89f751f..0cf6932ecdc 100644
> --- a/include/qobject/json-parser.h
> +++ b/include/qobject/json-parser.h
> @@ -31,8 +31,8 @@ typedef struct JSONMessageParser {
> void *opaque;
> JSONLexer lexer;
> JSONParserContext parser;
> - int brace_count;
> - int bracket_count;
> + unsigned int brace_count;
> + unsigned int bracket_count;
> GQueue tokens;
> uint64_t token_size;
> } JSONMessageParser;
> diff --git a/qobject/json-streamer.c b/qobject/json-streamer.c
> index f3dfdcaea12..b0bf2083ca6 100644
> --- a/qobject/json-streamer.c
> +++ b/qobject/json-streamer.c
> @@ -41,12 +41,18 @@ void json_message_process_token(JSONLexer *lexer, GString
> *input,
> parser->brace_count++;
> break;
> case JSON_RCURLY:
> + if (parser->brace_count <= 0) {
Why <= 0? parser->brace_count is unsigned now...
> + goto end_error_recovery;
> + }
> parser->brace_count--;
> break;
> case JSON_LSQUARE:
> parser->bracket_count++;
> break;
> case JSON_RSQUARE:
> + if (parser->bracket_count <= 0) {
Likewise.
> + goto end_error_recovery;
> + }
> parser->bracket_count--;
> break;
> case JSON_ERROR:
> @@ -56,6 +62,15 @@ void json_message_process_token(JSONLexer *lexer, GString
> *input,
> if (g_queue_is_empty(&parser->tokens)) {
> return;
> }
> + end_error_recovery:
> + /*
> + * Cause error recovery to end immediately.
> + * If not in error recovery, the parser will raise an error
> + * (due to JSON_ERROR or unexpected JSON_R{CURLY,SQUARE})
What do you mean by "due to JSON_ERROR"?
We get here on unmatched } or ]: that's the gotos
you add above.
We also get here on end of non-empty input: that's when the if right
above doesn't return.
> + * but error recovery won't be entered at all.
> + */
This is the only comment on error recovery. Would a comment giving a
brief overview on error recovery be useful?
> + parser->brace_count = 0;
> + parser->bracket_count = 0;
> break;
> default:
> break;
> @@ -83,9 +98,7 @@ void json_message_process_token(JSONLexer *lexer, GString
> *input,
>
> g_queue_push_tail(&parser->tokens, token);
>
> - if ((parser->brace_count > 0 || parser->bracket_count > 0)
> - && parser->brace_count >= 0 && parser->bracket_count >= 0
> - && type != JSON_END_OF_INPUT) {
> + if (parser->brace_count > 0 || parser->bracket_count > 0) {
> return;
> }
We get here when
* no lexical error, and
* no limits exceeded, and
* the curlies and squares are balanced, either in fact or because
we passed through end_error_recovery.
Good.
/* Process all tokens in the queue */
while (!g_queue_is_empty(&parser->tokens)) {
