[libvirt] [PATCH 03/18] cpu: Add NULL check in ppc64ModelCopy()

[Jiri Denemark]

On Tue, Aug 04, 2015 at 11:37:54 +0200, Andrea Bolognani wrote:
> ---
>  src/cpu/cpu_ppc64.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c
> index 05ff8f2..dd02a3f 100644
> --- a/src/cpu/cpu_ppc64.c
> +++ b/src/cpu/cpu_ppc64.c
> @@ -115,6 +115,9 @@ ppc64ModelCopy(const struct ppc64_model *model)
>  {
>      struct ppc64_model *copy;
>  
> +    if (!model)
> +        return NULL;
> +
>      if (VIR_ALLOC(copy) < 0 ||
>          VIR_STRDUP(copy->name, model->name) < 0) {
>          ppc64ModelFree(copy);

This doesn't seem to be really necessary since the function is not
called with model == NULL and I don't think that should change. If any
caller wants to pass NULL for a model, it should rather report an error
and return instead of trying to copy this NULL.

However, the only called of ppc64ModelCopy is pretty confusing:

    static struct ppc64_model *
    ppc64ModelFromCPU(const virCPUDef *cpu,
                      const struct ppc64_map *map)
    {
        struct ppc64_model *model;

        if (!(model = ppc64ModelFind(map, cpu->model))) {
            virReportError(VIR_ERR_INTERNAL_ERROR,
                           _("Unknown CPU model %s"), cpu->model);
            goto error;
        }

        if (!(model = ppc64ModelCopy(model)))
            goto error;

        return model;

     error:
        ppc64ModelFree(model);
        return NULL;
    }

It uses "model" for pointing to a model which must not be freed and also
to its copy which has to be freed. There's no bug here but I think
changing is a good idea :-)

Jirka