[libvirt] [PATCH 2/2] libxl: use init and dispose functions with libxl_physinfo

[Joao Martins]

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 <jfehlig at suse.com>
>>
>> Reviewed-by: Joao Martins <joao.m.martins at 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.martins at 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;
>>>  }
>>>
>