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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org