On 08/08/2018 07:03 AM, Markus Armbruster wrote:
Signed-off-by: Markus Armbruster <arm...@redhat.com>
---
  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.

Reviewed-by: Eric Blake <ebl...@redhat.com>

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

Reply via email to