On 08/30/12 16:06, Daniel Veillard wrote:
On Thu, Aug 30, 2012 at 03:51:54PM +0200, Peter Krempa wrote:
virDomainVcpuPinDefCopy when the control flow reaches out of memory
cleanup code, the flow would end in a infinite loop as the loop variable
wasn't decremented.
Also a dereference of NULL pointers was possible if allocation of the
Vcpu pinning definiton structure failed.
---
src/conf/domain_conf.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 224aec5..2dad64d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1496,7 +1496,7 @@ virDomainVcpuPinDefPtr *
virDomainVcpuPinDefCopy(virDomainVcpuPinDefPtr *src, int nvcpupin)
{
int i = 0;
- virDomainVcpuPinDefPtr *ret;
+ virDomainVcpuPinDefPtr *ret = NULL;
if (VIR_ALLOC_N(ret, nvcpupin) < 0) {
goto no_memory;
@@ -1514,11 +1514,15 @@ virDomainVcpuPinDefCopy(virDomainVcpuPinDefPtr *src, int nvcpupin)
return ret;
no_memory:
- while (i >= 0) {
- VIR_FREE(ret[i]->cpumask);
- VIR_FREE(ret[i]);
+ if (ret) {
+ for ( ; i >= 0; --i) {
+ if (ret[i]) {
+ VIR_FREE(ret[i]->cpumask);
+ VIR_FREE(ret[i]);
+ }
+ }
+ VIR_FREE(ret);
}
- VIR_FREE(ret);
virReportOOMError();
return NULL;
Whoops, ACK ! Please push
BTW I'm unclear what our prefered style is for
if (ret) {
...
VIR_FREE(ret);
This alternative saves the one function call and the identical check, so
I tend to use it rather than the other one
}
vs
if (ret) {
...
}
VIR_FREE(ret);
because that pattern appears twice in the replacement code.
To me that's fine, but since we avoid if (ret) VIR_FREE(ret);
I'm asking :-)
Daniel
Pushed. Thanks!
Peter
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list