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