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

Re: [libvirt] [PATCH 00/26] Resolve more Coverity issues




On 09/04/2014 06:26 PM, John Ferlan wrote:
> Sorry for the large dump, but before I got too involved in other things
> I figured I'd go through the list of the remaining 68 Coverity issues
> from the new version in order to reduce the pile. Many are benign, some
> seemingly false positives, and I think most are error paths. The one
> non error path that does stick out is the qemu_driver.c changes in the
> qemuDomainSetBlkioParameters() routine where 'param' and 'params' were
> used differently between LIVE and CONFIG. In particular, in CONFIG the
> use of 'params->field' instead of 'param->field'.
> 
> One that does bear looking at more closely and if someone has a better
> idea is avoiding a false positive resource_leak in remote_driver.c. I
> left a healthy comment in the code - you'll know when you see it.
> 
> These patches get the numbers down to 19 issues.  Of the remaining
> issues - some are related to Coverity thinking that 'mgetgroups' could
> return a negative value with an allocated groups structure (which I'm
> still scratching my head over).  There is also a few calls to
> virJSONValueObjectGetNumberUlong() in qemu_monitor_json.c that don't
> check status, but I'm not sure why - just didn't have the research
> cycles for that.
> 
> John Ferlan (26):
>   qemu_driver: Resolve Coverity COPY_PASTE_ERROR
>   remote_driver: Resolve Coverity RESOURCE_LEAK
>   storage: Resolve Coverity UNUSED_VALUE
>   vbox: Resolve Coverity UNUSED_VALUE
>   qemu: Resolve Coverity REVERSE_INULL
>   storage: Resolve Coverity OVERFLOW_BEFORE_WIDEN
>   virsh: Resolve Coverity DEADCODE
>   virfile: Resolve Coverity DEADCODE
>   virsh: Resolve Coverity DEADCODE
>   qemu: Resolve Coverity DEADCODE
>   tests: Resolve Coverity DEADCODE
>   virsh: Resolve Coverity DEADCODE
>   qemu: Resolve Coverity FORWARD_NULL
>   lxc: Resolve Coverity FORWARD_NULL
>   qemu: Resolve Coverity FORWARD_NULL
>   network: Resolve Coverity FORWARD_NULL
>   virstring: Resolve Coverity FORWARD_NULL
>   qemu: Resolve Coverity FORWARD_NULL
>   network_conf: Resolve Coverity FORWARD_NULL
>   qemu: Resolve Coverity NEGATIVE_RETURNS
>   nodeinfo: Resolve Coverity NEGATIVE_RETURNS
>   virsh: Resolve Coverity NEGATIVE_RETURNS
>   xen: Resolve Coverity NEGATIVE_RETURNS
>   qemu: Resolve Coverity NEGATIVE_RETURNS
>   qemu: Resolve Coverity NEGATIVE_RETURNS
>   libxl: Resolve Coverity NULL_RETURNS
> 
>  src/conf/network_conf.c            |  4 ++--
>  src/libxl/libxl_migration.c        |  1 -
>  src/lxc/lxc_driver.c               |  6 ++++--
>  src/network/leaseshelper.c         |  3 +--
>  src/nodeinfo.c                     |  2 +-
>  src/qemu/qemu_capabilities.c       |  2 +-
>  src/qemu/qemu_command.c            |  1 +
>  src/qemu/qemu_driver.c             | 26 +++++++++++++++-----------
>  src/qemu/qemu_migration.c          |  3 ++-
>  src/qemu/qemu_monitor_json.c       |  2 +-
>  src/qemu/qemu_process.c            |  5 +++--
>  src/remote/remote_driver.c         | 12 ++++++++++++
>  src/storage/storage_backend_disk.c |  2 +-
>  src/storage/storage_backend_fs.c   |  1 -
>  src/util/virfile.c                 |  5 ++---
>  src/util/virstring.c               |  3 +++
>  src/vbox/vbox_common.c             |  9 +++++++--
>  src/xen/xend_internal.c            |  3 ++-
>  tests/virstringtest.c              |  5 +++++
>  tools/virsh-domain.c               | 22 ++++++++--------------
>  tools/virsh-edit.c                 |  9 ---------
>  tools/virsh-interface.c            |  3 ---
>  tools/virsh-network.c              | 12 +++++-------
>  tools/virsh-nwfilter.c             |  3 ---
>  tools/virsh-pool.c                 |  3 ---
>  tools/virsh-snapshot.c             |  3 ---
>  26 files changed, 76 insertions(+), 74 deletions(-)
> 

Thanks for taking the time to review...

I have pushed those that were ACK'd:

1, 3-21, 23-26

Modifying 8 to remove the parentheses around the (errno == EINTR)

Modifying 9 to change the ternary into an if/else, restoring the typestr
= NULL; initializer. Also changed the commit message to match the change
made...

Modifying 21 to remove the "if (cpus < 0) VIR_FREE(cpus)" since the code
above had already handled clearing cpus

Leaving

2, 22

To revisit in a v2

John


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