On Wed, Jan 20, 2010 at 02:46:10AM +0100, Matthias Bolte wrote:
> @@ -792,16 +797,30 @@ qemudCapsInitGuest(virCapsPtr caps,
> */
> if (STREQ(info->arch, hostmachine) ||
> (STREQ(hostmachine, "x86_64") && STREQ(info->arch, "i686"))) {
> - const char *const kvmbins[] = { "/usr/bin/qemu-kvm", /* Fedora */
> - "/usr/bin/kvm" }; /* Upstream .spec */
> + if (access("/dev/kvm", F_OK) == 0) {
> + const char *const kvmbins[] = { "qemu-kvm", /* Fedora */
> + "kvm" }; /* Upstream .spec */
> +
> + for (i = 0; i < ARRAY_CARDINALITY(kvmbins); ++i) {
> + kvmbin = virFindFileInPath(kvmbins[i]);
> +
> + if (kvmbin == NULL || access(kvmbin, X_OK) != 0) {
> + VIR_FREE(kvmbin);
> + continue;
> + }
>
> - for (i = 0; i < ARRAY_CARDINALITY(kvmbins); ++i) {
> - if (access(kvmbins[i], X_OK) == 0 &&
> - access("/dev/kvm", F_OK) == 0) {
> haskvm = 1;
> - kvmbin = kvmbins[i];
> - if (!binary)
> - binary = kvmbin;
> +
> + if (binary == NULL) {
> + binary = strdup(kvmbin);
Why does this need to strdup(kvmbin), when virFindFileInPath() is
already returning a newly allocated string ? Seems like we could
just avoid that
> +
> + if (binary == NULL) {
> + virReportOOMError(NULL);
> + VIR_FREE(kvmbin);
> + return -1;
> + }
> + }
> +
> break;
> }
I think this loop is also leaking 'kvmbin', since it just
overrwrites 'kvmbin' on each iteration, the final cleanup
code will only free the last one that was allocated
> - return -1;
> + goto error;
> }
> }
>
> + VIR_FREE(binary);
> + VIR_FREE(kvmbin);
> +
> return 0;
> +
> +no_memory:
> + virReportOOMError(NULL);
> +
> +error:
> + VIR_FREE(binary);
> + VIR_FREE(kvmbin);
> + virCapabilitiesFreeMachines(machines, nmachines);
> +
> + return -1;
> }
>
Generally the patch looks OK though
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list