On mer, 2013-09-18 at 15:37 +1200, Matthew Daley wrote:
> Coverity-ID: 1055293
> Signed-off-by: Matthew Daley <mattjd@gmail.com>
> ---
> 
> diff --git a/tools/libxl/libxl_numa.c b/tools/libxl/libxl_numa.c
> index 20c99ac..f9944e0 100644
> --- a/tools/libxl/libxl_numa.c
> +++ b/tools/libxl/libxl_numa.c
> @@ -495,7 +495,8 @@ int libxl__get_numa_candidate(libxl__gc *gc,
>      libxl_bitmap_dispose(&suitable_nodemap);
>      libxl__numa_candidate_dispose(&new_cndt);
>      libxl_numainfo_list_free(ninfo, nr_nodes);
> -    libxl_cputopology_list_free(tinfo, nr_cpus);
> +    if (tinfo != NULL)
> +        libxl_cputopology_list_free(tinfo, nr_cpus);
>
I don't think this is necessary. Actually, although not wrong, it
deviates from the usual exiting pattern: init everything at the
beginning, free everything on the way out.

libxl_cputopology_list_free() is well capable of dealing with a
non-allocated tinfo, provided nr_cpus is 0 too. If I'm not mistaken,
that is exactly the case, all the times that tinfo is NULL, since:
 - it is initialized to 0 in libxl__get_numa_candidate();
 - its value is only altered, within libxl_get_cpu_topology(), in case 
   tinfo != NULL

So, really, I don't think we should apply this.

Also, if something like that would be necessary, why doing it only for
tinfo and not for ninfo as well? I'm not that into coverity, but I don't
think I see why it treats the two of them differently... :-O

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)