2009/11/25 Jim Fehlig <jfehlig@xxxxxxxxxx>:
> Commit 790f0b3057787bb64da8c46c111ff8d3eff7b2af causes contents of names
> array to be freed even on success, resulting in no listing of defined
> but inactive Xen domains. Patch below fixes it.
>
> Regards,
> Jim
>
Good catch, I just reviewed this commit to see if I've caused similar
bugs elsewhere, but this seems to be the only one.
> Index: libvirt-0.7.4/src/xen/xend_internal.c
> ===================================================================
> --- libvirt-0.7.4.orig/src/xen/xend_internal.c
> +++ libvirt-0.7.4/src/xen/xend_internal.c
> @@ -4693,13 +4693,14 @@ xenDaemonListDefinedDomains(virConnectPt
> }
>
> if (ret >= maxnames)
> - break;
> + goto out;
> }
>
> error:
> for (i = 0; i < ret; ++i)
> VIR_FREE(names[i]);
>
> +out:
> sexpr_free(root);
> return(ret);
> }
Your patch doesn't fix the problem in all situations. If maxnames is
larger than the actual number of domains then goto out is never
executed.
I also forgot to set ret to -1 after freeing the names, this needs to
be fixed too.
I propose the attached patch to solve this issues.
Matthias
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index d61e9e6..a0c7d77 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -4696,12 +4696,17 @@ xenDaemonListDefinedDomains(virConnectPtr conn, char **const names, int maxnames
break;
}
+cleanup:
+ sexpr_free(root);
+ return(ret);
+
error:
for (i = 0; i < ret; ++i)
VIR_FREE(names[i]);
- sexpr_free(root);
- return(ret);
+ ret = -1;
+
+ goto cleanup;
}
/**
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list