[libvirt] [PATCH 1/2] nwfilter: address coverity findings

[Eric Blake]

On 04/26/2012 01:17 PM, Stefan Berger wrote:
> This patch addresses the following coverity findings:
> 
> /libvirt/src/conf/nwfilter_params.c:157:
> deref_parm: Directly dereferencing parameter "val".
> 
> /libvirt/src/conf/nwfilter_params.c:473:
> negative_returns: Using variable "iterIndex" as an index to array
> "res->iter".
> 
> /libvirt/src/nwfilter/nwfilter_ebiptables_driver.c:2891:
> unchecked_value: No check of the return value of "virAsprintf(&protostr,
> "-d 01:80:c2:00:00:00 ")".
> 
> /libvirt/src/nwfilter/nwfilter_ebiptables_driver.c:2894:
> unchecked_value: No check of the return value of "virAsprintf(&protostr,
> "-p 0x%04x ", l3_protocols[protoidx].attr)".
> 
> /libvirt/src/nwfilter/nwfilter_ebiptables_driver.c:3590:
> var_deref_op: Dereferencing null variable "inst".
> 
> ---
>  src/conf/nwfilter_params.c                |    5 ++++-
>  src/nwfilter/nwfilter_ebiptables_driver.c |   10 +++++++---
>  2 files changed, 11 insertions(+), 4 deletions(-)

Nice little grab-bag of patches.

>          if (virNWFilterVarCombIterAddVariable(&res->iter[iterIndex],
> Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
> ===================================================================
> --- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
> +++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
> @@ -2878,6 +2878,7 @@ ebtablesCreateTmpSubChain(ebiptablesRule
>      char chainPrefix = (incoming) ? CHAINPREFIX_HOST_IN_TEMP
>                                    : CHAINPREFIX_HOST_OUT_TEMP;
>      char *protostr = NULL;
> +    int r = 0;

No need for r.

> 
>      PRINT_ROOT_CHAIN(rootchain, chainPrefix, ifname);
>      PRINT_CHAIN(chain, chainPrefix, ifname,
> @@ -2888,14 +2889,14 @@ ebtablesCreateTmpSubChain(ebiptablesRule
>          protostr = strdup("");
>          break;
>      case L2_PROTO_STP_IDX:
> -        virAsprintf(&protostr, "-d " NWFILTER_MAC_BGA " ");
> +        r = virAsprintf(&protostr, "-d " NWFILTER_MAC_BGA " ");

Here, I'd just write:

ignore_value(virAsprintf(...));

>          break;
>      default:
> -        virAsprintf(&protostr, "-p 0x%04x ", l3_protocols[protoidx].attr);
> +        r = virAsprintf(&protostr, "-p 0x%04x ",

and here,

> l3_protocols[protoidx].attr);
>          break;
>      }
> 
> -    if (!protostr) {

because virAsprintf guarantees that if it returned < 0, then protostr is
NULL and you have an OOM situation.  In other words, our code is already
correct, and we just need the ignore_value() wrapper to shut up the
static analyzer.

The other hunks were right.

ACK with the cleanup mentioned, I'm okay if you push without posting a v2.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120426/b4406faa/attachment-0001.sig>