[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 05/21] 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()
---
 daemon/libvirtd.c                     |   13 ++++++---
 src/conf/domain_conf.c                |    7 ++++-
 src/network/bridge_driver.c           |    6 +++-
 src/qemu/qemu_conf.c                  |    9 ++++++-
 src/security/security_selinux.c       |    2 -
 src/storage/storage_backend_logical.c |    5 +++-
 src/util/util.c                       |    6 +++-
 src/util/util.h                       |   44 ++++++++++++++++----------------
 src/vbox/vbox_tmpl.c                  |    4 ++-
 src/xen/xm_internal.c                 |    6 +++-
 tests/qemuargv2xmltest.c              |    2 -
 11 files changed, 64 insertions(+), 40 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 78dfb2d..fa473ce 100644
--- 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 0dd2b3f..a6d8e07 100644
--- 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 95bc810..2a6a7ab 100644
--- 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 158e9a3..951a6c6 100644
--- 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 7e0f71a..6a03af7 100644
--- 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 4389120..eac3917 100644
--- 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
 
@@ -341,7 +342,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 98f8a14..08070da 100644
--- 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 8679636..3ef26e6 100644
--- 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 4741c57..aecda23 100644
--- 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 732b2d3..b52f66e 100644
--- 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 1b16aa9..5602df0 100644
--- 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)
-- 
1.6.2.5


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]