Re: [libvirt] [PATCH 2/2] libxl: use init and dispose functions with libxl_physinfo
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 <jfehlig(a)suse.com>
Reviewed-by: Joao Martins <joao.m.martins(a)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 :-).
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;
> }
>