qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] qobject: json-streamer: Remove double test


From: Simran Singhal
Subject: Re: [PATCH] qobject: json-streamer: Remove double test
Date: Thu, 2 Apr 2020 22:11:32 +0530



On Thu, Apr 2, 2020 at 6:42 PM Eric Blake <address@hidden> wrote:
On 4/2/20 7:13 AM, Simran Singhal wrote:
> Remove the duplicate test "parser->bracket_count >= 0".
>
> Signed-off-by: Simran Singhal <address@hidden>
> ---
>   qobject/json-streamer.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/qobject/json-streamer.c b/qobject/json-streamer.c
> index 47dd7ea576..ef48185283 100644
> --- a/qobject/json-streamer.c
> +++ b/qobject/json-streamer.c
> @@ -85,7 +85,7 @@ void json_message_process_token(JSONLexer *lexer, GString *input,
>       g_queue_push_tail(&parser->tokens, token);
>   

Adding some context:

>       if ((parser->brace_count > 0 || parser->bracket_count > 0)
> -        && parser->bracket_count >= 0 && parser->bracket_count >= 0) {
> +        && parser->bracket_count >= 0) {
>           return;
>       }
>   
>     json = json_parser_parse(parser->tokens, parser->ap, &err);
>     parser->tokens = NULL;
>
> out_emit:

This code was rewritten in commit 8d3265b3.  Prior to that, it read:


     if (parser->brace_count < 0 ||
         parser->bracket_count < 0 ||
         (parser->brace_count == 0 &&
          parser->bracket_count == 0)) {
         json = json_parser_parse(parser->tokens, parser->ap, &err);
         parser->tokens = NULL;
         goto out_emit;
     }

     return;

out_emit:

Obviously, the goal of the rewrite was to convert:

if (cond) {
   do stuff
} else {
   return
}
more stuff

into the more legible

if (!cond) {
   return
}
do stuff
more stuff

Let's re-read my original review:

https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg03017.html

> 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
>

What I missed was the typo: we checked bracket >= 0 twice, instead of
the intended brace >= 0 && bracket >= 0.  This needs a v2.

Hello Eric,

Thanks for the explanation.
I'll send v2 with the required changes.

--
Simran
 

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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]