[PATCH v2 2/7] hyperv: implement nodeGetFreeMemory

[Matt Coleman]

> On Oct 15, 2020, at 8:56 AM, Michal Privoznik <mprivozn at redhat.com> wrote:
> 
> On 10/13/20 7:13 AM, Matt Coleman wrote:
>> +    if (hypervGetWmiClass(Win32_OperatingSystem, &operatingSystem) < 0 ||
>> +        !operatingSystem) {
>> +        virReportError(VIR_ERR_INTERNAL_ERROR,
>> +                       _("Could not get free memory for host %s"),
>> +                       conn->uri->server);
> 
> IIUC, hypervGetWmiClass() reports an error (in fact more accurate one) on failure. So this overwrite doesn't look good. Also, there is no point calling free if @operatingSystem is NULL ;-)

Thanks for pointing all of that out.

> 
> How about this squashed in?
> 
> diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c
> index 0f6d3cb946..195cb4997a 100644
> --- a/src/hyperv/hyperv_driver.c
> +++ b/src/hyperv/hyperv_driver.c
> @@ -1403,20 +1403,19 @@ hypervNodeGetFreeMemory(virConnectPtr conn)
>     Win32_OperatingSystem *operatingSystem = NULL;
>     g_auto(virBuffer) query = { g_string_new(WIN32_OPERATINGSYSTEM_WQL_SELECT), 0 };
> 
> -    if (hypervGetWmiClass(Win32_OperatingSystem, &operatingSystem) < 0 ||
> -        !operatingSystem) {
> +    if (hypervGetWmiClass(Win32_OperatingSystem, &operatingSystem) < 0)
> +        return 0;
> +
> +    if (!operatingSystem) {
>         virReportError(VIR_ERR_INTERNAL_ERROR,
>                        _("Could not get free memory for host %s"),
>                        conn->uri->server);
> -        goto cleanup;
> +        return 0;
>     }
> 
>     /* Return free memory in bytes */
>     res = operatingSystem->data.common->FreePhysicalMemory * 1024;
> -
> -    cleanup:
>     hypervFreeObject(priv, (hypervObject *) operatingSystem);
> -
>     return res;
> }

This is a solid improvement.

The pattern `if (hypervGetWmiClass() < 0 || !foo)` is used in several 
other places. Would you like me to fix those other occurrences?

-- 
Matt