[libvirt] [PATCH] Enforce return check on virAsprintf() calls

[Eric Blake]

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

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