Re: [Qemu-devel] [PATCH 49/56] json: Streamline json_message_process_token()

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 08/08/2018 07:03 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>   qobject/json-streamer.c | 13 +++++--------
>>   1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/qobject/json-streamer.c b/qobject/json-streamer.c
>> index 810aae521f..954bf9d468 100644
>> --- a/qobject/json-streamer.c
>> +++ b/qobject/json-streamer.c
>> @@ -99,16 +99,13 @@ 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 ||
>
> Old: if we are unbalanced (more right tokens read than left)...
>
>> -        (parser->brace_count == 0 &&
>> -         parser->bracket_count == 0)) {
>
> ...or if we uniformly ended nesting,...
>
>> -        json = json_parser_parse(parser->tokens, parser->ap, &err);
>
> ...then parse (to either diagnose the unbalance, or to see if the
> balanced construct is valid), with weird flow control that skips over
> an early return.
>
> Or put another way, if we invert the condition, we find the cases
> where we want an early return instead of parsing (and can thus use
> that to get rid of an unsightly goto over a single early return).
>
> Applying deMorgan's rules:
>
> !(brace < 0 || bracket < 0 || (brace == 0 && bracket == 0))
> !(brace < 0) && !(bracket < 0) && !(brace == 0 && bracket == 0)
> brace >= 0 && bracket >= 0 && (!(brace == 0) || !(bracket == 0))
> brace >= 0 && bracket >= 0 && (brace != 0 || bracket != 0)
>
> But based on what we learned in the first two conjunctions, we can
> rewrite the third:
>
> brace >= 0 && bracket >= 0 && (brace > 0 || bracket > 0)
>
> and then commute the logic:
>
> (brace > 0 || bracket > 0) && brace >= 0 && bracket >= 0
>
>> -        parser->tokens = NULL;
>> -        goto out_emit;
>> +    if ((parser->brace_count > 0 || parser->bracket_count > 0)
>> +        && parser->bracket_count >= 0 && parser->bracket_count >= 0) {
>
> So the new condition is correct, and reads as:
>
> If either struct is still awaiting closure, and both structs have not
> gone unbalanced, then early exit.
>
> It was not intuitive, but stepping through the logic shows it is identical.

My first version had a "simpler" condition there.  My test cases proved
it wrong %-}

> Reviewed-by: Eric Blake <address@hidden>

Thanks!