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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list