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@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list