[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 12/28] libxl: only free cputopology if it was allocated in libxl__get_numa_candidate



On mer, 2013-09-18 at 15:37 +1200, Matthew Daley wrote:
> Coverity-ID: 1055293
> Signed-off-by: Matthew Daley <mattjd@xxxxxxxxx>
> ---
> 
> 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)

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.