Daniel Veillard wrote:
Okay, that's something i promised last week, but it has some significant changes: - cleanup the XPath methods to access the XML informations, reusing the existing methods from xml.h - make string fields from __lxc_vm_def dynamic (except the UUID) - fix what looked like a funny leak situation when vm def structures were deallocated (see changes around lxcFreeVM/lxcFreeVMs/lxcFreeVMDef), i hope i didn't overlooked something but valgrind seems happy now leak wise. Overall it looks a bit simpler to me :-)
I agree :-)
Valgrind only report left is ==1== Invalid write of size 4 ==1== at 0x470C8C: lxcCheckContainerSupport (lxc_driver.c:91) ==1== Address 0xFFFFFFFFFFFFFFEC is not stack'd, malloc'd or (recently) free'd ==1== ==1== Process terminating with default action of signal 11 (SIGSEGV) ==1== Access not within mapped region at address 0xFFFFFFFFFFFFFFEC ==1== at 0x470C8C: lxcCheckContainerSupport (lxc_driver.c:91)
which is related to the probe code of the driver --------------------------- stack = malloc(getpagesize() * 4); if(!stack) { DEBUG0("Unable to allocate stack"); rc = -1; goto check_complete; }
childStack = stack + (getpagesize() * 4);
cpid = clone(lxcDummyChild, childStack, flags, NULL); --------------------------- I would assume it's valgrind being a bit pedantic because we pass the address just over the allocated stack limit, which should be fine since stack is addressed in predecrementing mode (BTW isn't that a portability bug in the code stack direction should probably be checked no ?).
This sounds like a pretty uncommon case - only the HP PA-RISC that I've found. This could certainly be addressed though...
Still maybe it's a good idea to pass a pointer in the allocated area ...
This could be done with a small loss (looks like 16 bytes) of stack space.
Daniel
void lxcFreeVMs(lxc_vm_t *vms) *************** void lxcFreeVMs(lxc_vm_t *vms) *** 859,865 **** while (curVm) { lxcFreeVM(curVm); nextVm = curVm->next; - free(curVm); curVm = nextVm; } } [...] --- 819,825 ---- void lxcFreeVM(lxc_vm_t *vm) { lxcFreeVMDef(vm->def); ! free(vm); }
This breaks doesn't it? After calling lxcFreeVM(), curVm is no longer valid since it was free()'d. Need to pull out the next pointer before lxcFreeVM(). -- Best Regards, Dave Leskovec IBM Linux Technology Center Open Virtualization