Annotate many methods with ATTRIBUTE_RETURN_CHECK & fix problems

Nearly all of the methods in src/util/util.h have error codes that
must be checked by the caller to correct detect & report failure.
Add ATTRIBUTE_RETURN_CHECK to ensure compile time validation of
this

* daemon/libvirtd.c: Add explicit check on return value of virAsprintf
* src/conf/domain_conf.c: Add missing check on virParseMacAddr return
  value status & report error
* src/network/bridge_driver.c: Add missing OOM check on virAsprintf
  and report error
* src/qemu/qemu_conf.c: Add missing check on virParseMacAddr return
  value status & report error
* src/security/security_selinux.c: Remove call to virRandomInitialize
  that's done in libvirt.c already
* src/storage/storage_backend_logical.c: Add check & log on virRun
  return status
* src/util/util.c: Add missing checks on virAsprintf/Run status
* src/util/util.h: Annotate all methods with ATTRIBUTE_RETURN_CHECK
  if they return an error status code
* src/vbox/vbox_tmpl.c: Add missing check on virParseMacAddr
* src/xen/xm_internal.c: Add missing checks on virAsprintf
* tests/qemuargv2xmltest.c: Remove bogus call to virRandomInitialize()

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 03d091a6f64fe9cc7ca7c7ce76dab84d0d7561cf..7e0697528cdf5f5b425fa04f8c982f6144b723c4 100644 (file)
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -752,13 +752,16 @@ static int qemudInitPaths(struct qemud_server *server,
             goto snprintf_error;
     }
 
-    if (server->privileged)
-        server->logDir = strdup (LOCAL_STATE_DIR "/log/libvirt");
-    else
-        virAsprintf(&server->logDir, "%s/.libvirt/log", dir_prefix);
+    if (server->privileged) {
+        if (!(server->logDir = strdup (LOCAL_STATE_DIR "/log/libvirt")))
+            virReportOOMError(NULL);
+    } else {
+        if (virAsprintf(&server->logDir, "%s/.libvirt/log", dir_prefix) < 0)
+            virReportOOMError(NULL);
+    }
 
     if (server->logDir == NULL)
-        virReportOOMError(NULL);
+        goto cleanup;
 
     ret = 0;
 

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0dd2b3f68f7eea9ec66f7e8aed8ae43531e01fcf..a6d8e07c12f232cefe2fafd82e3dd6a35e3a5968 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1109,7 +1109,12 @@ virDomainNetDefParseXML(virConnectPtr conn,
     }
 
     if (macaddr) {
-        virParseMacAddr((const char *)macaddr, def->mac);
+        if (virParseMacAddr((const char *)macaddr, def->mac) < 0) {
+            virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                                 _("unable to parse mac address '%s'"),
+                                 (const char *)macaddr);
+            goto error;
+        }
     } else {
         virCapabilitiesGenerateMac(caps, def->mac);
     }

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index bc241ef4dd666d7fa45a4b23766a50440cb87233..837bd765015e2519dbd88cbb62b5f6087c6c9791 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -149,7 +149,10 @@ networkFindActiveConfigs(struct network_driver *driver) {
 #ifdef __linux__
                 char *pidpath;
 
-                virAsprintf(&pidpath, "/proc/%d/exe", obj->dnsmasqPid);
+                if (virAsprintf(&pidpath, "/proc/%d/exe", obj->dnsmasqPid) < 0) {
+                    virReportOOMError(NULL);
+                    goto cleanup;
+                }
                 if (virFileLinkPointsTo(pidpath, DNSMASQ) == 0)
                     obj->dnsmasqPid = -1;
                 VIR_FREE(pidpath);
@@ -157,6 +160,7 @@ networkFindActiveConfigs(struct network_driver *driver) {
             }
         }
 
+    cleanup:
         virNetworkObjUnlock(obj);
     }
 }

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index a095cb75c5a2492246f5200dc38936e9683b10b0..59873d7d50b5adc84f9624b47c8d774f8ae017f0 100644 (file)
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -2841,7 +2841,14 @@ qemuParseCommandLineNet(virConnectPtr conn,
     for (i = 0 ; i < nkeywords ; i++) {
         if (STREQ(keywords[i], "macaddr")) {
             genmac = 0;
-            virParseMacAddr(values[i], def->mac);
+            if (virParseMacAddr(values[i], def->mac) < 0) {
+                qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+                                 _("unable to parse mac address '%s'"),
+                                 values[i]);
+                virDomainNetDefFree(def);
+                def = NULL;
+                goto cleanup;
+            }
         } else if (STREQ(keywords[i], "model")) {
             def->model = values[i];
             values[i] = NULL;

diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 7e0f71aeebeec05dfe9946cb3a057e5cf761bc3b..6a03af78a33d4af49b05d92803b10cf254049a76 100644 (file)
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -108,8 +108,6 @@ SELinuxInitialize(virConnectPtr conn)
     char *ptr = NULL;
     int fd = 0;
 
-    virRandomInitialize(time(NULL) ^ getpid());
-
     fd = open(selinux_virtual_domain_context_path(), O_RDONLY);
     if (fd < 0) {
         virReportSystemError(conn, errno,

diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c
index 91726cd5be5dcb51adb4b8aaac25ac99e38c82ca..5eebfac92d2d3750248f5c8aadf6efec2741da5e 100644 (file)
--- a/src/storage/storage_backend_logical.c
+++ b/src/storage/storage_backend_logical.c
@@ -36,6 +36,7 @@
 #include "storage_conf.h"
 #include "util.h"
 #include "memory.h"
+#include "logging.h"
 
 #define VIR_FROM_THIS VIR_FROM_STORAGE
 
@@ -336,7 +337,9 @@ virStorageBackendLogicalFindPoolSources(virConnectPtr conn,
      * that might be hanging around, so if this fails for some reason, the
      * worst that happens is that scanning doesn't pick everything up
      */
-    virRun(conn, scanprog, &exitstatus);
+    if (virRun(conn, scanprog, &exitstatus) < 0) {
+        VIR_WARN0("Failure when running vgscan to refresh physical volumes");
+    }
 
     memset(&sourceList, 0, sizeof(sourceList));
     sourceList.type = VIR_STORAGE_POOL_LOGICAL;

diff --git a/src/util/util.c b/src/util/util.c
index 98f8a14e395a605104f22bcd9d5415459833d1f6..08070da3738373f940571c6cb25641a913e6585c 100644 (file)
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1257,7 +1257,8 @@ int virFileOpenTtyAt(const char *ptmx ATTRIBUTE_UNUSED,
 char* virFilePid(const char *dir, const char* name)
 {
     char *pidfile;
-    virAsprintf(&pidfile, "%s/%s.pid", dir, name);
+    if (virAsprintf(&pidfile, "%s/%s.pid", dir, name) < 0)
+        return NULL;
     return pidfile;
 }
 
@@ -2108,7 +2109,8 @@ void virFileWaitForDevices(virConnectPtr conn)
      * If this fails for any reason, we still have the backup of polling for
      * 5 seconds for device nodes.
      */
-    virRun(conn, settleprog, &exitstatus);
+    if (virRun(conn, settleprog, &exitstatus) < 0)
+    {}
 }
 #else
 void virFileWaitForDevices(virConnectPtr conn ATTRIBUTE_UNUSED) {}

diff --git a/src/util/util.h b/src/util/util.h
index 8679636bde74ad5e0faaa3ea320f380d0b321808..3ef26e63f4d60fb90eb8e8e3d9c80e7565b2221f 100644 (file)
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -41,8 +41,8 @@ enum {
     VIR_EXEC_CLEAR_CAPS = (1 << 2),
 };
 
-int virSetNonBlock(int fd);
-int virSetCloseExec(int fd);
+int virSetNonBlock(int fd) ATTRIBUTE_RETURN_CHECK;
+int virSetCloseExec(int fd) ATTRIBUTE_RETURN_CHECK;
 
 /* This will execute in the context of the first child
  * after fork() but before execve() */
@@ -57,7 +57,7 @@ int virExecDaemonize(virConnectPtr conn,
                      int flags,
                      virExecHook hook,
                      void *data,
-                     char *pidfile);
+                     char *pidfile) ATTRIBUTE_RETURN_CHECK;
 int virExecWithHook(virConnectPtr conn,
                     const char *const*argv,
                     const char *const*envp,
@@ -69,7 +69,7 @@ int virExecWithHook(virConnectPtr conn,
                     int flags,
                     virExecHook hook,
                     void *data,
-                    char *pidfile);
+                    char *pidfile) ATTRIBUTE_RETURN_CHECK;
 int virExec(virConnectPtr conn,
             const char *const*argv,
             const char *const*envp,
@@ -78,14 +78,14 @@ int virExec(virConnectPtr conn,
             int infd,
             int *outfd,
             int *errfd,
-            int flags);
-int virRun(virConnectPtr conn, const char *const*argv, int *status);
+            int flags) ATTRIBUTE_RETURN_CHECK;
+int virRun(virConnectPtr conn, const char *const*argv, int *status) ATTRIBUTE_RETURN_CHECK;
 
-int virFileReadLimFD(int fd, int maxlen, char **buf);
+int virFileReadLimFD(int fd, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK;
 
-int virFileReadAll(const char *path, int maxlen, char **buf);
+int virFileReadAll(const char *path, int maxlen, char **buf) ATTRIBUTE_RETURN_CHECK;
 
-int virFileWriteStr(const char *path, const char *str);
+int virFileWriteStr(const char *path, const char *str) ATTRIBUTE_RETURN_CHECK;
 
 int virFileMatchesNameSuffix(const char *file,
                              const char *name,
@@ -95,28 +95,28 @@ int virFileHasSuffix(const char *str,
                      const char *suffix);
 
 int virFileStripSuffix(char *str,
-                       const char *suffix);
+                       const char *suffix) ATTRIBUTE_RETURN_CHECK;
 
 int virFileLinkPointsTo(const char *checkLink,
                         const char *checkDest);
 
 int virFileResolveLink(const char *linkpath,
-                       char **resultpath);
+                       char **resultpath) ATTRIBUTE_RETURN_CHECK;
 
 char *virFindFileInPath(const char *file);
 
 int virFileExists(const char *path);
 
-int virFileMakePath(const char *path);
+int virFileMakePath(const char *path) ATTRIBUTE_RETURN_CHECK;
 
 int virFileBuildPath(const char *dir,
                      const char *name,
                      const char *ext,
                      char *buf,
-                     unsigned int buflen);
+                     unsigned int buflen) ATTRIBUTE_RETURN_CHECK;
 
 int virFileAbsPath(const char *path,
-                   char **abspath);
+                   char **abspath) ATTRIBUTE_RETURN_CHECK;
 
 int virFileOpenTty(int *ttymaster,
                    char **ttyName,
@@ -129,13 +129,13 @@ int virFileOpenTtyAt(const char *ptmx,
 char* virFilePid(const char *dir,
                  const char *name);
 int virFileWritePidPath(const char *path,
-                        pid_t pid);
+                        pid_t pid) ATTRIBUTE_RETURN_CHECK;
 int virFileWritePid(const char *dir,
                     const char *name,
-                    pid_t pid);
+                    pid_t pid) ATTRIBUTE_RETURN_CHECK;
 int virFileReadPid(const char *dir,
                    const char *name,
-                   pid_t *pid);
+                   pid_t *pid) ATTRIBUTE_RETURN_CHECK;
 int virFileDeletePid(const char *dir,
                      const char *name);
 
@@ -167,7 +167,7 @@ int virMacAddrCompare (const char *mac1, const char *mac2);
 void virSkipSpaces(const char **str);
 int virParseNumber(const char **str);
 int virAsprintf(char **strp, const char *fmt, ...)
-    ATTRIBUTE_FMT_PRINTF(2, 3);
+    ATTRIBUTE_FMT_PRINTF(2, 3) 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)
@@ -179,7 +179,7 @@ char *virStrcpy(char *dest, const char *src, size_t destbytes)
 #define VIR_MAC_STRING_BUFLEN VIR_MAC_BUFLEN * 3
 
 int virParseMacAddr(const char* str,
-                    unsigned char *addr);
+                    unsigned char *addr) ATTRIBUTE_RETURN_CHECK;
 void virFormatMacAddr(const unsigned char *addr,
                       char *str);
 void virGenerateMacAddr(const unsigned char *prefix,
@@ -233,13 +233,13 @@ char *virGetUserName(virConnectPtr conn,
                      uid_t uid);
 int virGetUserID(virConnectPtr conn,
                  const char *name,
-                 uid_t *uid);
+                 uid_t *uid) ATTRIBUTE_RETURN_CHECK;
 int virGetGroupID(virConnectPtr conn,
                   const char *name,
-                  gid_t *gid);
+                  gid_t *gid) ATTRIBUTE_RETURN_CHECK;
 #endif
 
-int virRandomInitialize(unsigned int seed);
+int virRandomInitialize(unsigned int seed) ATTRIBUTE_RETURN_CHECK;
 int virRandom(int max);
 
 #ifdef HAVE_MNTENT_H

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 4741c573117c3c045a12cc1dccbfd62f24c42e43..aecda23bbb512e6a4027b0bdc269abd1853987f7 100644 (file)
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -2219,7 +2219,9 @@ static char *vboxDomainDumpXML(virDomainPtr dom, int flags) {
                                      MACAddress[4], MACAddress[5], MACAddress[6], MACAddress[7],
                                      MACAddress[8], MACAddress[9], MACAddress[10], MACAddress[11]);
 
-                            virParseMacAddr(macaddr, def->nets[netAdpIncCnt]->mac);
+                            /* XXX some real error handling here some day ... */
+                            if (virParseMacAddr(macaddr, def->nets[netAdpIncCnt]->mac) < 0)
+                            {}
 
                             netAdpIncCnt++;
 

diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 732b2d3c787a65a540e3e30ae39deb04fa2c61bd..b52f66ee3aad0a2d573792df4a9b61ccf49950d6 100644 (file)
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -3035,14 +3035,16 @@ xenXMDomainBlockPeek (virDomainPtr dom,
 static char *xenXMAutostartLinkName(virDomainPtr dom)
 {
     char *ret;
-    virAsprintf(&ret, "/etc/xen/auto/%s", dom->name);
+    if (virAsprintf(&ret, "/etc/xen/auto/%s", dom->name) < 0)
+        return NULL;
     return ret;
 }
 
 static char *xenXMDomainConfigName(virDomainPtr dom)
 {
     char *ret;
-    virAsprintf(&ret, "/etc/xen/%s", dom->name);
+    if (virAsprintf(&ret, "/etc/xen/%s", dom->name) < 0)
+        return NULL;
     return ret;
 }
 

diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
index 1b16aa91b5f02aa9891f15ffc907ee5475ef6f26..5602df0414f4149a7eafb08154b62428d286c51f 100644 (file)
--- a/tests/qemuargv2xmltest.c
+++ b/tests/qemuargv2xmltest.c
@@ -109,8 +109,6 @@ mymain(int argc, char **argv)
     if (!abs_srcdir)
         abs_srcdir = getcwd(cwd, sizeof(cwd));
 
-    virRandomInitialize(0);
-
     if ((driver.caps = testQemuCapsInit()) == NULL)
         return EXIT_FAILURE;
     if((driver.stateDir = strdup("/nowhere")) == NULL)