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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list