7:35 a.m.
On Wed, May 02, 2012 at 01:53:36PM -0400, William Jon McCann wrote:
+static int migrateProfile(void)
+{
+ char *old_base = NULL;
+ char *updated = NULL;
+ char *home = NULL;
+ char *xdg_dir = NULL;
+ char *config_dir = NULL;
+ const char *config_home;
+ int ret = -1;
+ mode_t old_umask;
+
+ if (!(home = virGetUserDirectory(geteuid())))
+ goto cleanup;
+
+ if (virAsprintf(&old_base, "%s/.libvirt", home) < 0) {
+ goto cleanup;
+ }
+
+ /* if the new directory is there or the old one is not: do nothing */
+ if (!(config_dir = virGetUserConfigDirectory(geteuid())))
+ goto cleanup;
+
+ if (!virFileIsDir(old_base) || virFileExists(config_dir)) {
This is missing an assignment 'ret = 0', since this is a success
(no-op) case, rather than a failure case.
+ goto cleanup;
+ }
+static char *virGetXDGDirectory(uid_t uid, const char *xdgenvname, const char
*xdgdefdir)
+{
+ char *path = NULL;
+ virBuffer buf = VIR_BUFFER_INITIALIZER;
+
+ if (uid == getuid ())
+ path = getenv(xdgenvname);
+
+ if (path && path[0]) {
+ virBufferAsprintf(&buf, "%s/libvirt/", path);
+ } else {
+ char *home;
+ home = virGetUserEnt(uid, VIR_USER_ENT_DIRECTORY);
+ virBufferAsprintf(&buf, "%s/%s/libvirt/", home, xdgdefdir);
+ VIR_FREE(home);
+ }
+ VIR_FREE(path);
Opps, getenv() returns a string that should not be freed.
Also we can simplify this by just using virAsprintf()
since there's only one printf required here
+
+ if (virBufferError(&buf)) {
+ virBufferFreeAndReset(&buf);
+ virReportOOMError();
+ return NULL;
+ }
+
+ return virBufferContentAndReset(&buf);
+}
+
+char *virGetUserConfigDirectory(uid_t uid)
+{
+ return virGetXDGDirectory(uid, "XDG_CONFIG_HOME", ".config");
+}
+
+char *virGetUserCacheDirectory(uid_t uid)
+{
+ return virGetXDGDirectory(uid, "XDG_CACHE_HOME", ".cache");
+}
+
Basically I applied the following diff ontop of your patch and it then
worked as expected for me
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index bb25678..67248b3 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -770,6 +770,7 @@ static int migrateProfile(void)
goto cleanup;
if (!virFileIsDir(old_base) || virFileExists(config_dir)) {
+ ret = 0;
goto cleanup;
}
diff --git a/src/util/util.c b/src/util/util.c
index 447b7b7..b47e3d8 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -2312,29 +2312,27 @@ char *virGetUserDirectory(uid_t uid)
static char *virGetXDGDirectory(uid_t uid, const char *xdgenvname, const char
*xdgdefdir)
{
- char *path = NULL;
- virBuffer buf = VIR_BUFFER_INITIALIZER;
+ const char *path = NULL;
+ char *ret = NULL;
+ char *home = virGetUserEnt(uid, VIR_USER_ENT_DIRECTORY);
- if (uid == getuid ())
+ if (uid == getuid())
path = getenv(xdgenvname);
if (path && path[0]) {
- virBufferAsprintf(&buf, "%s/libvirt/", path);
+ if (virAsprintf(&ret, "%s/libvirt/", path) < 0)
+ goto no_memory;
} else {
- char *home;
- home = virGetUserEnt(uid, VIR_USER_ENT_DIRECTORY);
- virBufferAsprintf(&buf, "%s/%s/libvirt/", home, xdgdefdir);
- VIR_FREE(home);
- }
- VIR_FREE(path);
-
- if (virBufferError(&buf)) {
- virBufferFreeAndReset(&buf);
- virReportOOMError();
- return NULL;
+ if (virAsprintf(&ret, "%s/%s/libvirt/", home, xdgdefdir) < 0)
+ goto no_memory;
}
- return virBufferContentAndReset(&buf);
+cleanup:
+ VIR_FREE(home);
+ return ret;
+no_memory:
+ virReportOOMError();
+ goto cleanup;
}
char *virGetUserConfigDirectory(uid_t uid)
@@ -2349,8 +2347,7 @@ char *virGetUserCacheDirectory(uid_t uid)
char *virGetUserRuntimeDirectory(uid_t uid)
{
- char *path = NULL;
- virBuffer buf = VIR_BUFFER_INITIALIZER;
+ const char *path = NULL;
if (uid == getuid ())
path = getenv("XDG_RUNTIME_DIR");
@@ -2358,16 +2355,12 @@ char *virGetUserRuntimeDirectory(uid_t uid)
if (!path || !path[0]) {
return virGetUserCacheDirectory(uid);
} else {
- virBufferAsprintf(&buf, "%s/libvirt/", path);
- VIR_FREE(path);
-
- if (virBufferError(&buf)) {
- virBufferFreeAndReset(&buf);
+ char *ret;
+ if (virAsprintf(&ret, "%s/libvirt/", path) < 0) {
virReportOOMError();
return NULL;
}
-
- return virBufferContentAndReset(&buf);
+ return ret;
}
}
I would ACK this patch with those changes applied. Having said that I
think I would prefer to wait until after the 0.9.12 release is cut
before applying this, since I'm afraid there may be some unexpected
consequences. Waiting till after 0.9.13 gives us a month to test it
in GIT master before it gets into a release.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|