[Libvirt-cim] [PATCH 7/7] Changes to 3 of 3 from code review

[Jincheng Miao]

----- Original Message -----
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/Virt_VirtualSystemManagementService.c | 38
>  ++++++++++++++++++++++++++-----
>  1 file changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/src/Virt_VirtualSystemManagementService.c
> b/src/Virt_VirtualSystemManagementService.c
> index 9634481..b478d41 100644
> --- a/src/Virt_VirtualSystemManagementService.c
> +++ b/src/Virt_VirtualSystemManagementService.c
> @@ -1841,23 +1841,42 @@ static const char *input_rasd_to_vdev(CMPIInstance
> *inst,
>  static const char *controller_rasd_to_vdev(CMPIInstance *inst,
>                                             struct virt_device *dev)
>  {
> -        const char *val;
> +        const char *val = NULL;
> +        const char *msg = NULL;
> +        int ret;
>  
> -        if (cu_get_str_prop(inst, "Type", &val) != CMPI_RC_OK) {
> -                CU_DEBUG("ControllerRASD Type field not valid");
> +        if (cu_get_str_prop(inst, "ResourceSubType", &val) != CMPI_RC_OK) {
> +                CU_DEBUG("ControllerRASD ResourceSubType field not valid");
>                  goto out;
>          }
>          dev->dev.controller.type = strdup(val);
>  
> +        if (cu_get_u32_prop(inst, "Index",
> +                            &dev->dev.controller.index) != CMPI_RC_OK) {
> +                CU_DEBUG("ControllerRASD Index field not valid");
> +                goto out;
> +        }
> +
>          if (cu_get_str_prop(inst, "Model", &val) != CMPI_RC_OK) {
>                  CU_DEBUG("Invalid value for Model in ControllerRASD");
>                  goto out;
>          }
>          dev->dev.controller.model = strdup(val);
>  
> +        ret = asprintf(&dev->id, "controller:%s:%s",
> +                       dev->dev.controller.type,
> +                       dev->dev.controller.index);
> +        if (ret == -1) {
> +                msg = "Failed to create controller string";
> +                goto out;
> +        }
> +
> +        msg = rasd_to_device_address(inst, &dev->dev.controller.address);
> +
> +
>   out:
>  
> -        return NULL;
> +        return ;

I am not clear why here is just 'return' ? IMHO, if this function doesn't
return anything, should it be defined returning void?
Otherwise return NULL is more safe, that is because if someone wanna reference
this return value, 'return' will give him a ramdom value(from a place of stack),
That will be SEGV.

>  }
>  
>  static const char *_sysvirt_rasd_to_vdev(CMPIInstance *inst,
> @@ -2131,13 +2150,20 @@ static const char *classify_resources(CMPIArray
> *resources,
>                                             ns,
>                                             p_error);
>                  } else if (type == CIM_RES_TYPE_CONTROLLER) {
> +                        struct virt_device dev;
> +                        int dcount = count + domain->dev_controller_ct;
> +
> +                        memset(&dev, 0, sizeof(dev));
>                          msg = rasd_to_vdev(inst,
>                                             domain,
> -
> &domain->dev_controller[domain->dev_controller_ct],
> +                                           &dev,
>                                             ns,
>                                             p_error);
>                          if (msg == NULL)
> -                                domain->dev_controller_ct += 1;
> +                                msg = add_device_nodup(&dev,
> +                                                 domain->dev_controller,
> +                                                 dcount,
> +
> &domain->dev_controller_ct);
>                  }
>  
>                  if (msg != NULL)
> --
> 1.8.5.3
> 
> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim
>