[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v1 33/37] Adapt to VIR_ALLOC and virAsprintf in src/xenapi/*



On 07/04/2013 02:06 PM, Michal Privoznik wrote:
> ---
>  src/xenapi/xenapi_driver.c | 29 ++++++++---------------------
>  src/xenapi/xenapi_utils.c  | 11 ++++-------
>  2 files changed, 12 insertions(+), 28 deletions(-)
> 
> diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
> index cc8da49..869ec76 100644
> --- a/src/xenapi/xenapi_driver.c
> +++ b/src/xenapi/xenapi_driver.c
> @@ -409,12 +403,9 @@ xenapiNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info)
>      if (xen_host_cpu_get_all(session, &host_cpu_set)) {
>          host_cpu = host_cpu_set->contents[0];
>          xen_host_cpu_get_modelname(session, &modelname, host_cpu);
> -        if (!virStrncpy(info->model, modelname, LIBVIRT_MODELNAME_LEN - 1, LIBVIRT_MODELNAME_LEN)) {
> -            virReportOOMError();
> -            xen_host_cpu_set_free(host_cpu_set);
> -            VIR_FREE(modelname);
> -            return -1;
> -        }
> +        ignore_value(virStrncpy(info->model, modelname,
> +                                LIBVIRT_MODELNAME_LEN - 1,
> +                                LIBVIRT_MODELNAME_LEN));
>          xen_host_cpu_get_speed(session, &mhz, host_cpu);
>          info->mhz = (unsigned long)mhz;
>          info->cpus = host_cpu_set->size;

I think this hunk should be a separate commit, since you didn't add OOM error
reporting to virStrncpy.

> @@ -545,22 +543,21 @@ createVMRecordFromXml(virConnectPtr conn, virDomainDefPtr def,
>              char *mac;
>  
>              if (VIR_ALLOC_N(mac, VIR_MAC_STRING_BUFLEN) < 0)
> -                goto error_cleanup;
> +                goto error;
>              virMacAddrFormat(&def->nets[i]->mac, mac);
>  
>              if (createVifNetwork(conn, *vm, device_number,
>                                   def->nets[i]->data.bridge.brname,
>                                   mac) < 0) {
>                  VIR_FREE(mac);
> -                goto error_cleanup;
> +                virReportOOMError();

This can result in double OOM Error reporting if createVifNetwork fails on
virAsprintf, we should use virAsprintfQuiet there instead.

> +                goto error;
>              }
>              device_number++;
>          }
>      }
>      return 0;
>  
> -  error_cleanup:
> -    virReportOOMError();
>    error:
>      xen_vm_record_free(*record);
>      return -1;
> 

ACK

Jan


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]