On 02/08/2017 04:17 PM, Jim Fehlig wrote: > Joao Martins wrote: >> On 02/02/2017 10:39 PM, Jim Fehlig wrote: >>> The typical pattern when calling libxl functions that populate a >>> structure is >>> >>> libxl_foo foo; >>> libxl_foo_init(&foo); >>> libxl_get_foo(ctx, &foo); >>> ... >>> libxl_foo_dispose(&foo); >>> >>> Fix several instances of libxl_physinfo missing the init and >>> dispose calls. >> Indeed, >> >>> Signed-off-by: Jim Fehlig <jfeh...@suse.com> >> >> Reviewed-by: Joao Martins <joao.m.mart...@oracle.com> >> >> See also one comment/nit below, perhaps one libxl_physinfo_init could be >> moved >> slightly up.. >> >>> [...] >> >>> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c >>> index 3a69720..8951bef 100644 >>> --- a/src/libxl/libxl_driver.c >>> +++ b/src/libxl/libxl_driver.c >>> @@ -4286,6 +4286,7 @@ libxlNodeGetFreeMemory(virConnectPtr conn) >>> if (virNodeGetFreeMemoryEnsureACL(conn) < 0) >>> goto cleanup; >>> >>> + libxl_physinfo_init(&phy_info); >> >> .. namely here? That is before virNodeGetFreeMemoryEnsureACL. > > Nice catch. Moved as suggested in my local branch. > > Any other comments on this small series? Would be nice to get these bug fixes > committed :-). > Nope, looks all good to me:
Acked-by: Joao Martins <joao.m.mart...@oracle.com> > Regards, > Jim > >> >> Not that it matters much, as init/dispose in this case just zeroes out >> phy_info >> region. But just perhaps consistency as you would end disposing an non >> initialized object. >> >>> if (libxl_get_physinfo(cfg->ctx, &phy_info)) { >>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s", >>> _("libxl_get_physinfo_info failed")); >>> @@ -4295,6 +4296,7 @@ libxlNodeGetFreeMemory(virConnectPtr conn) >>> ret = phy_info.free_pages * cfg->verInfo->pagesize; >>> >>> cleanup: >>> + libxl_physinfo_dispose(&phy_info); >>> virObjectUnref(cfg); >>> return ret; >>> } >>> > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list