[libvirt] [PATCH] Enforce return check on virAsprintf() calls
Way back when I started making changes for Coverity messages my first set were to a bunch of CHECKED_RETURN errors. In particular virAsprintf() had a few callers that Coverity noted didn't check their return (although some did check if the buffer being printed to was NULL or not). It was suggested at the time as a further patch an ATTRIBUTE_RETURN_CHECK should be added to virAsprintf(), see: https://www.redhat.com/archives/libvir-list/2013-January/msg00120.html This patch does that and fixes two more instances not found by Coverity that failed the check. --- src/util/virutil.h | 3 ++- src/xen/xend_internal.c | 6 ++---- tests/networkxml2conftest.c | 5 +++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/util/virutil.h b/src/util/virutil.h index 5a08c81..c386d24 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -200,7 +200,8 @@ int virParseNumber(const char **str); int virParseVersionString(const char *str, unsigned long *version, bool allowMissing); int virAsprintf(char **strp, const char *fmt, ...) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 3); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 3) + ATTRIBUTE_RETURN_CHECK; int virVasprintf(char **strp, const char *fmt, va_list list) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 0); char *virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index b03b7bc..3036cbb 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -2738,10 +2738,8 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, virDevicePCIAddress PCIAddr; PCIAddr = dev->data.hostdev->source.subsys.u.pci; - virAsprintf(&target, "PCI device: %.4x:%.2x:%.2x", PCIAddr.domain, - PCIAddr.bus, PCIAddr.slot); - - if (target == NULL) { + if (virAsprintf(&target, "PCI device: %.4x:%.2x:%.2x", + PCIAddr.domain, PCIAddr.bus, PCIAddr.slot) < 0) { virReportOOMError(); goto cleanup; } diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index 3979540..fd24f74 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -102,8 +102,9 @@ testDnsmasqLeaseFileName(const char *netname) { char *leasefile; - virAsprintf(&leasefile, "/var/lib/libvirt/dnsmasq/%s.leases", - netname); + if (virAsprintf(&leasefile, "/var/lib/libvirt/dnsmasq/%s.leases", + netname) < 0) + return NULL; return leasefile; } -- 1.7.11.7
On 01/30/2013 01:43 PM, John Ferlan wrote:
Way back when I started making changes for Coverity messages my first set were to a bunch of CHECKED_RETURN errors. In particular virAsprintf() had a few callers that Coverity noted didn't check their return (although some did check if the buffer being printed to was NULL or not).
It was suggested at the time as a further patch an ATTRIBUTE_RETURN_CHECK should be added to virAsprintf(), see:
https://www.redhat.com/archives/libvir-list/2013-January/msg00120.html
This patch does that and fixes two more instances not found by Coverity that failed the check.
int virAsprintf(char **strp, const char *fmt, ...) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 3); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 3) + ATTRIBUTE_RETURN_CHECK; int virVasprintf(char **strp, const char *fmt, va_list list) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 0);
VirVasprintf also needs this.
- virAsprintf(&leasefile, "/var/lib/libvirt/dnsmasq/%s.leases", - netname); + if (virAsprintf(&leasefile, "/var/lib/libvirt/dnsmasq/%s.leases", + netname) < 0) + return NULL;
return leasefile;
Since we do nothing other than return NULL on error, but leasefile is already NULL on error, this one is simpler if we just use ignore_value(). ACK. I'm squashing this, then pushing. diff --git i/src/util/virerror.c w/src/util/virerror.c index 500c32b..301a1ac 100644 --- i/src/util/virerror.c +++ w/src/util/virerror.c @@ -1,7 +1,7 @@ /* * virerror.c: error handling and reporting code for libvirt * - * Copyright (C) 2006, 2008-2012 Red Hat, Inc. + * Copyright (C) 2006, 2008-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -647,7 +647,7 @@ virRaiseErrorFull(const char *filename ATTRIBUTE_UNUSED, } else { va_list ap; va_start(ap, fmt); - virVasprintf(&str, fmt, ap); + ignore_value(virVasprintf(&str, fmt, ap)); va_end(ap); } diff --git i/src/util/virutil.h w/src/util/virutil.h index c386d24..4201aa1 100644 --- i/src/util/virutil.h +++ w/src/util/virutil.h @@ -1,7 +1,7 @@ /* * virutil.h: common, generic utility functions * - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright (C) 2006, 2007 Binary Karma * Copyright (C) 2006 Shuveb Hussain * @@ -203,7 +203,8 @@ int virAsprintf(char **strp, const char *fmt, ...) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 3) ATTRIBUTE_RETURN_CHECK; int virVasprintf(char **strp, const char *fmt, va_list list) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 0); + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_FMT_PRINTF(2, 0) + ATTRIBUTE_RETURN_CHECK; char *virStrncpy(char *dest, const char *src, size_t n, size_t destbytes) ATTRIBUTE_RETURN_CHECK; char *virStrcpy(char *dest, const char *src, size_t destbytes) diff --git i/tests/networkxml2conftest.c w/tests/networkxml2conftest.c index fd24f74..88016ee 100644 --- i/tests/networkxml2conftest.c +++ w/tests/networkxml2conftest.c @@ -102,10 +102,8 @@ testDnsmasqLeaseFileName(const char *netname) { char *leasefile; - if (virAsprintf(&leasefile, "/var/lib/libvirt/dnsmasq/%s.leases", - netname) < 0) - return NULL; - + ignore_value(virAsprintf(&leasefile, "/var/lib/libvirt/dnsmasq/%s.leases", + netname)); return leasefile; } diff --git i/tools/virsh-domain.c w/tools/virsh-domain.c index 004fac4..151b349 100644 --- i/tools/virsh-domain.c +++ w/tools/virsh-domain.c @@ -1,7 +1,7 @@ /* * virsh-domain.c: Commands to manage domain * - * Copyright (C) 2005, 2007-2012 Red Hat, Inc. + * Copyright (C) 2005, 2007-2013 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -9011,8 +9011,7 @@ vshCompleteXMLFromDomain(vshControl *ctl, virDomainPtr dom, char *oldXML, } /* Get all possible devices */ - virAsprintf(&xpath, "/domain/devices/%s", node->name); - if (!xpath) { + if (virAsprintf(&xpath, "/domain/devices/%s", node->name) < 0) { virReportOOMError(); goto cleanup; } -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
John Ferlan