[libvirt] [PATCH 05/21] Annotate many methods with ATTRIBUTE_RETURN_CHECK & fix problems
Daniel P. Berrange
berrange at redhat.com
Fri Oct 23 13:05:34 UTC 2009
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
More information about the libvir-list
mailing list