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

[Matthew Daley <mattjd@xxxxxxxxx>]

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