On Wed, Aug 08, 2012 at 03:03:51PM +0100, Daniel P. Berrange wrote:
On Sun, Aug 05, 2012 at 10:04:35AM +0530, M. Mohan Kumar wrote:
@@ -2575,9 +2577,43 @@ error: return NULL; }
+/* + * Invokes the Proxy Helper with one of the socketpair as its parameter + * + */ +static int qemuInvokeProxyHelper(const char *emulator, int sock, const char *path) +{ +#define HELPER "virtfs-proxy-helper" + int ret_val, status; + virCommandPtr cmd; + char *helper, *dname; + + dname = dirname(strdup(emulator));
Missing check for NULL from strdup()
+ if (virAsprintf(&helper, "%s/%s", dname, HELPER) < 0) { + VIR_FREE(dname); + virReportOOMError(); + return -1;
You must VIR_FORCE_CLOSE(sock) here, since other exit paths will close it
+ }
I think this is slightly bogus - and it'd be better to do virFindFileInPath(HELPER), and if that doesn't work then also look in /usr/libexec/$HELPER.
Actually I take this back. You are doing what I originally suggested you do, which is correct. Looking in $PATH would be wrong since it might find a proxy that's from a different version of QEMU. We do need to take account of fact that QEMU might be in $prefix/libexec, instead of $prefix/bin. Also Unfortunately dirname is not thread-safe since it can return a statically allocated string - which must not be free()d, so we can't use that. So I think I'd build the path slightly differently: char *tmp, *prefix; if (!(prefix = strdup(emulator))) { virReportOOMError(); return -1; } if (tmp = strstr(prefix, "/bin/") || tmp = strstr(prefix, "/libexec/") { *tmp = '\0'; } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to determine prefix from %s"), emulator); VIR_FREE(prefix); } if (virAsprintf(&path, "%s/bin/" HELPER, prefix) < 0) { VIR_FREE(prefix); return -1; } VIR_FREE(prefix); 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 :|