All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>, qemu-devel@nongnu.org
Cc: kwolf@redhat.com, mdroth@linux.vnet.ibm.com, aliguori@amazon.com
Subject: Re: [Qemu-devel] [PATCH 10/10] qapi: Clean up null checking in generated visitors
Date: Thu, 06 Feb 2014 20:10:41 -0700	[thread overview]
Message-ID: <52F44EB1.8010306@redhat.com> (raw)
In-Reply-To: <1391697000-5855-11-git-send-email-armbru@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 1559 bytes --]

On 02/06/2014 07:30 AM, Markus Armbruster wrote:
> Visitors get passed a pointer to the visited object.  The generated
> visitors try to cope with this pointer being null in some places, for
> instance like this:
> 
>     visit_start_optional(m, obj ? &(*obj)->has_name : NULL, "name", &err);
> 
> visit_start_optional() passes its second argument to Visitor method
> start_optional.  Two out of two methods dereference it
> unconditionally.

Let's see if I can find them without Coverity's help...

opts-visitor.c:opts_start_optional
qmp-input-visitor.c:qmp_input_start_optional
string-input-visitor.c:parse_start_optional

Your counting is off :)  All three unconditionally dereference.

[While at it, this code style from parse_start_optional, and similar in
other locations, is rather verbose:

    if (!siv->string) {
        *present = false;
        return;
    }

    *present = true;
}

Shorter would be:

    *present = !!siv->string;
}

but that doesn't affect this patch]

> 
> I fail to see how hits pointer could legitimately be null.
> 
> All this useless null checking is highly redundant, which Coverity
> duly reports.  About 200 times.
> 
> Remove the useless null checks.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi-visit.py | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  reply	other threads:[~2014-02-07  3:10 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-06 14:29 [Qemu-devel] [PATCH 00/10] qapi: Test coverage & clean up generated code Markus Armbruster
2014-02-06 14:29 ` [Qemu-devel] [PATCH 01/10] tests/qapi-schema: Actually check successful QMP command response Markus Armbruster
2014-02-07  2:09   ` Eric Blake
2014-02-07  7:37     ` Markus Armbruster
2014-02-06 14:29 ` [Qemu-devel] [PATCH 02/10] tests/qapi-schema: Cover optional command arguments Markus Armbruster
2014-02-07  2:30   ` Eric Blake
2014-02-06 14:29 ` [Qemu-devel] [PATCH 03/10] tests/qapi-schema: Cover simple argument types Markus Armbruster
2014-02-07  2:32   ` Eric Blake
2014-02-06 14:29 ` [Qemu-devel] [PATCH 04/10] tests/qapi-schema: Cover anonymous union types Markus Armbruster
2014-02-07  2:35   ` Eric Blake
2014-02-06 14:29 ` [Qemu-devel] [PATCH 05/10] tests/qapi-schema: Cover complex types with base Markus Armbruster
2014-02-07  2:38   ` Eric Blake
2014-02-06 14:29 ` [Qemu-devel] [PATCH 06/10] tests/qapi-schema: Cover union " Markus Armbruster
2014-02-07  2:40   ` Eric Blake
2014-02-06 14:29 ` [Qemu-devel] [PATCH 07/10] tests/qapi-schema: Cover flat union types Markus Armbruster
2014-02-07  2:51   ` Eric Blake
2014-02-06 14:29 ` [Qemu-devel] [PATCH 08/10] qapi: Drop nonsensical header guard in generated qapi-visit.c Markus Armbruster
2014-02-07  2:52   ` Eric Blake
2014-02-06 14:29 ` [Qemu-devel] [PATCH 09/10] qapi: Drop unused code in qapi-commands.py Markus Armbruster
2014-02-07  2:56   ` Eric Blake
2014-02-06 14:30 ` [Qemu-devel] [PATCH 10/10] qapi: Clean up null checking in generated visitors Markus Armbruster
2014-02-07  3:10   ` Eric Blake [this message]
2014-02-07  7:34     ` Markus Armbruster
2014-02-07 12:42   ` Paolo Bonzini
2014-02-07 14:23     ` Markus Armbruster
2014-02-07 14:45       ` Paolo Bonzini
2014-02-10 13:29       ` Markus Armbruster
2014-02-11  9:06         ` Paolo Bonzini
2014-02-11 12:35           ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52F44EB1.8010306@redhat.com \
    --to=eblake@redhat.com \
    --cc=aliguori@amazon.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.