[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 Wed, Sep 18, 2013 at 7:57 PM, Dario Faggioli <dario.faggioli@xxxxxxxxxx> wrote: > 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 Argh, you are absolutely correct. > > So, really, I don't think we should apply this. Agreed. Please ignore this patch, and sorry for the noise. > > 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 Not sure. Probably because if ninfo == NULL, we return directly out of the function instead of goto-ing the cleanup. - Matthew _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |