[libvirt] [PATCH 01/20] Remove PATH_MAX sized stack allocations related to virFileBuildPath
Matthias Bolte
matthias.bolte at googlemail.com
Sun Apr 3 09:21:14 UTC 2011
Make virFileBuildPath operate on the heap instead of the stack. It
allocates a buffer instead of expecting a preexisting buffer.
---
src/conf/nwfilter_conf.c | 21 +++++++--------------
src/conf/storage_conf.c | 44 +++++++++++++++-----------------------------
src/util/util.c | 29 +++++++++++++++--------------
src/util/util.h | 8 +++-----
src/xen/xen_inotify.c | 16 +++++++++-------
src/xen/xm_internal.c | 33 +++++++++++++++++----------------
6 files changed, 66 insertions(+), 85 deletions(-)
diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c
index 13b5b38..df8e20f 100644
--- a/src/conf/nwfilter_conf.c
+++ b/src/conf/nwfilter_conf.c
@@ -2484,7 +2484,7 @@ virNWFilterLoadAllConfigs(virConnectPtr conn,
}
while ((entry = readdir(dir))) {
- char path[PATH_MAX];
+ char *path;
virNWFilterObjPtr nwfilter;
if (entry->d_name[0] == '.')
@@ -2493,17 +2493,16 @@ virNWFilterLoadAllConfigs(virConnectPtr conn,
if (!virFileHasSuffix(entry->d_name, ".xml"))
continue;
- if (virFileBuildPath(configDir, entry->d_name,
- NULL, path, PATH_MAX) < 0) {
- virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
- _("config filename '%s/%s' is too long"),
- configDir, entry->d_name);
+ if (!(path = virFileBuildPath(configDir, entry->d_name, NULL))) {
+ virReportOOMError();
continue;
}
nwfilter = virNWFilterObjLoad(conn, nwfilters, entry->d_name, path);
if (nwfilter)
virNWFilterObjUnlock(nwfilter);
+
+ VIR_FREE(path);
}
closedir(dir);
@@ -2523,7 +2522,6 @@ virNWFilterObjSaveDef(virNWFilterDriverStatePtr driver,
if (!nwfilter->configFile) {
int err;
- char path[PATH_MAX];
if ((err = virFileMakePath(driver->configDir))) {
virReportSystemError(err,
@@ -2532,13 +2530,8 @@ virNWFilterObjSaveDef(virNWFilterDriverStatePtr driver,
return -1;
}
- if (virFileBuildPath(driver->configDir, def->name, ".xml",
- path, sizeof(path)) < 0) {
- virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("cannot construct config file path"));
- return -1;
- }
- if (!(nwfilter->configFile = strdup(path))) {
+ if (!(nwfilter->configFile = virFileBuildPath(driver->configDir,
+ def->name, ".xml"))) {
virReportOOMError();
return -1;
}
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 13a3622..5a069f5 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -1473,8 +1473,8 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools,
}
while ((entry = readdir(dir))) {
- char path[PATH_MAX];
- char autostartLink[PATH_MAX];
+ char *path;
+ char *autostartLink;
virStoragePoolObjPtr pool;
if (entry->d_name[0] == '.')
@@ -1483,19 +1483,15 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools,
if (!virFileHasSuffix(entry->d_name, ".xml"))
continue;
- if (virFileBuildPath(configDir, entry->d_name,
- NULL, path, PATH_MAX) < 0) {
- virStorageReportError(VIR_ERR_INTERNAL_ERROR,
- _("Config filename '%s/%s' is too long"),
- configDir, entry->d_name);
+ if (!(path = virFileBuildPath(configDir, entry->d_name, NULL))) {
+ virReportOOMError();
continue;
}
- if (virFileBuildPath(autostartDir, entry->d_name,
- NULL, autostartLink, PATH_MAX) < 0) {
- virStorageReportError(VIR_ERR_INTERNAL_ERROR,
- _("Autostart link path '%s/%s' is too long"),
- autostartDir, entry->d_name);
+ if (!(autostartLink = virFileBuildPath(autostartDir, entry->d_name,
+ NULL))) {
+ virReportOOMError();
+ VIR_FREE(path);
continue;
}
@@ -1503,6 +1499,9 @@ virStoragePoolLoadAllConfigs(virStoragePoolObjListPtr pools,
autostartLink);
if (pool)
virStoragePoolObjUnlock(pool);
+
+ VIR_FREE(path);
+ VIR_FREE(autostartLink);
}
closedir(dir);
@@ -1520,7 +1519,6 @@ virStoragePoolObjSaveDef(virStorageDriverStatePtr driver,
if (!pool->configFile) {
int err;
- char path[PATH_MAX];
if ((err = virFileMakePath(driver->configDir))) {
virReportSystemError(err,
@@ -1529,26 +1527,14 @@ virStoragePoolObjSaveDef(virStorageDriverStatePtr driver,
return -1;
}
- if (virFileBuildPath(driver->configDir, def->name, ".xml",
- path, sizeof(path)) < 0) {
- virStorageReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("cannot construct config file path"));
- return -1;
- }
- if (!(pool->configFile = strdup(path))) {
+ if (!(pool->configFile = virFileBuildPath(driver->configDir,
+ def->name, ".xml"))) {
virReportOOMError();
return -1;
}
- if (virFileBuildPath(driver->autostartDir, def->name, ".xml",
- path, sizeof(path)) < 0) {
- virStorageReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("cannot construct "
- "autostart link path"));
- VIR_FREE(pool->configFile);
- return -1;
- }
- if (!(pool->autostartLink = strdup(path))) {
+ if (!(pool->autostartLink = virFileBuildPath(driver->autostartDir,
+ def->name, ".xml"))) {
virReportOOMError();
VIR_FREE(pool->configFile);
return -1;
diff --git a/src/util/util.c b/src/util/util.c
index 43794b1..31feecc 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -1841,23 +1841,24 @@ cleanup:
return err;
}
-/* Build up a fully qualfiied path for a config file to be
+/* Build up a fully qualified path for a config file to be
* associated with a persistent guest or network */
-int virFileBuildPath(const char *dir,
- const char *name,
- const char *ext,
- char *buf,
- unsigned int buflen)
+char *
+virFileBuildPath(const char *dir, const char *name, const char *ext)
{
- if ((strlen(dir) + 1 + strlen(name) + (ext ? strlen(ext) : 0) + 1) >= (buflen-1))
- return -1;
+ char *path;
- strcpy(buf, dir);
- strcat(buf, "/");
- strcat(buf, name);
- if (ext)
- strcat(buf, ext);
- return 0;
+ if (ext == NULL) {
+ if (virAsprintf(&path, "%s/%s", dir, name) < 0) {
+ return NULL;
+ }
+ } else {
+ if (virAsprintf(&path, "%s/%s%s", dir, name, ext) < 0) {
+ return NULL;
+ }
+ }
+
+ return path;
}
diff --git a/src/util/util.h b/src/util/util.h
index 7b7722a..d320c40 100644
--- a/src/util/util.h
+++ b/src/util/util.h
@@ -146,11 +146,9 @@ int virDirCreate(const char *path, mode_t mode, uid_t uid, gid_t gid,
unsigned int flags) ATTRIBUTE_RETURN_CHECK;
int virFileMakePath(const char *path) ATTRIBUTE_RETURN_CHECK;
-int virFileBuildPath(const char *dir,
- const char *name,
- const char *ext,
- char *buf,
- unsigned int buflen) ATTRIBUTE_RETURN_CHECK;
+char *virFileBuildPath(const char *dir,
+ const char *name,
+ const char *ext) ATTRIBUTE_RETURN_CHECK;
int virFileAbsPath(const char *path,
char **abspath) ATTRIBUTE_RETURN_CHECK;
diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c
index 7d4ba4c..5a997e6 100644
--- a/src/xen/xen_inotify.c
+++ b/src/xen/xen_inotify.c
@@ -387,7 +387,7 @@ xenInotifyOpen(virConnectPtr conn,
{
DIR *dh;
struct dirent *ent;
- char path[PATH_MAX];
+ char *path;
xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
if (priv->configDir) {
@@ -414,19 +414,21 @@ xenInotifyOpen(virConnectPtr conn,
continue;
/* Build the full file path */
- if ((strlen(priv->configDir) + 1 +
- strlen(ent->d_name) + 1) > PATH_MAX)
- continue;
- strcpy(path, priv->configDir);
- strcat(path, "/");
- strcat(path, ent->d_name);
+ if (!(path = virFileBuildPath(priv->configDir, ent->d_name, NULL))) {
+ virReportOOMError();
+ closedir(dh);
+ return -1;
+ }
if (xenInotifyAddDomainConfigInfo(conn, path) < 0 ) {
virXenInotifyError(VIR_ERR_INTERNAL_ERROR,
"%s", _("Error adding file to config list"));
closedir(dh);
+ VIR_FREE(path);
return -1;
}
+
+ VIR_FREE(path);
}
closedir(dh);
}
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index 7f73588..9225808 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -358,7 +358,7 @@ int xenXMConfigCacheRefresh (virConnectPtr conn) {
while ((ent = readdir(dh))) {
struct stat st;
- char path[PATH_MAX];
+ char *path;
/*
* Skip a bunch of crufty files that clearly aren't config files
@@ -387,15 +387,16 @@ int xenXMConfigCacheRefresh (virConnectPtr conn) {
continue;
/* Build the full file path */
- if ((strlen(priv->configDir) + 1 + strlen(ent->d_name) + 1) > PATH_MAX)
- continue;
- strcpy(path, priv->configDir);
- strcat(path, "/");
- strcat(path, ent->d_name);
+ if (!(path = virFileBuildPath(priv->configDir, ent->d_name, NULL))) {
+ virReportOOMError();
+ closedir(dh);
+ return -1;
+ }
/* Skip anything which isn't a file (takes care of scripts/ subdir */
if ((stat(path, &st) < 0) ||
(!S_ISREG(st.st_mode))) {
+ VIR_FREE(path);
continue;
}
@@ -404,6 +405,8 @@ int xenXMConfigCacheRefresh (virConnectPtr conn) {
if (xenXMConfigCacheAddFile(conn, path) < 0) {
/* Ignoring errors, since alot of stuff goes wrong in /etc/xen */
}
+
+ VIR_FREE(path);
}
/* Reap all entries which were not changed, by comparing
@@ -1046,10 +1049,11 @@ int xenXMDomainCreate(virDomainPtr domain) {
* Create a config file for a domain, based on an XML
* document describing its config
*/
-virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) {
+virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml)
+{
virDomainPtr ret;
- char filename[PATH_MAX];
- const char * oldfilename;
+ char *filename;
+ const char *oldfilename;
virDomainDefPtr def = NULL;
xenXMConfCachePtr entry = NULL;
xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
@@ -1130,16 +1134,11 @@ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) {
entry = NULL;
}
- if ((strlen(priv->configDir) + 1 + strlen(def->name) + 1) > PATH_MAX) {
- xenXMError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("config file name is too long"));
+ if (!(filename = virFileBuildPath(priv->configDir, def->name, NULL))) {
+ virReportOOMError();
goto error;
}
- strcpy(filename, priv->configDir);
- strcat(filename, "/");
- strcat(filename, def->name);
-
if (xenXMConfigSaveFile(conn, filename, def) < 0)
goto error;
@@ -1172,9 +1171,11 @@ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) {
ret = virGetDomain(conn, def->name, def->uuid);
xenUnifiedUnlock(priv);
+ VIR_FREE(filename);
return (ret);
error:
+ VIR_FREE(filename);
VIR_FREE(entry);
virDomainDefFree(def);
xenUnifiedUnlock(priv);
--
1.7.0.4
More information about the libvir-list
mailing list