Message ID | 1477070247-32185-1-git-send-email-wei.liu2@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On Fri, 2016-10-21 at 18:17 +0100, Wei Liu wrote: > Clang complains nr_dom_vcpus may be used uninitialized after > 4a6070ea9. > > The real issue is vinfo can be NULL and nr_dom_vcpus remains > uninitialized if previous call fails. > If it were me doing this, I'd just have initialized nr_dom_vcpus to 0. This is what I've always done when using libxl_numainfo_list_free(), which is similar to this (then how did I miss doing it here? Oh, well... :-/) And since we're initializing vinfo to NULL already, initializing nr_dom_vcpus as well makes things look more uniform. That being said, this amounts to personal taste, you're the maintainer, and it's not that I can't stand this solution, so: > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > Reviewed-by: Dario Faggioli <dario.faggioli@citrix.com> Thanks and Regards, Dario
On Fri, Oct 21, 2016 at 07:26:16PM +0200, Dario Faggioli wrote: > On Fri, 2016-10-21 at 18:17 +0100, Wei Liu wrote: > > Clang complains nr_dom_vcpus may be used uninitialized after > > 4a6070ea9. > > > > The real issue is vinfo can be NULL and nr_dom_vcpus remains > > uninitialized if previous call fails. > > > If it were me doing this, I'd just have initialized nr_dom_vcpus to 0. > > This is what I've always done when using libxl_numainfo_list_free(), > which is similar to this (then how did I miss doing it here? Oh, > well... :-/) > > And since we're initializing vinfo to NULL already, initializing > nr_dom_vcpus as well makes things look more uniform. > Right. I think this is a better idea. I will send out another version of this patch. Wei.
diff --git a/tools/libxl/libxl_numa.c b/tools/libxl/libxl_numa.c index fd64c22..0bdac2a 100644 --- a/tools/libxl/libxl_numa.c +++ b/tools/libxl/libxl_numa.c @@ -254,7 +254,8 @@ static int nr_vcpus_on_nodes(libxl__gc *gc, libxl_cputopology *tinfo, next: libxl_cpupoolinfo_dispose(&cpupool_info); - libxl_vcpuinfo_list_free(vinfo, nr_dom_vcpus); + if (vinfo) + libxl_vcpuinfo_list_free(vinfo, nr_dom_vcpus); } libxl_bitmap_dispose(&dom_nodemap);
Clang complains nr_dom_vcpus may be used uninitialized after 4a6070ea9. The real issue is vinfo can be NULL and nr_dom_vcpus remains uninitialized if previous call fails. Instead of initializing nr_dom_vcpus to 0, check if vinfo is NULL before calling the free function, because that function can't handle NULL. That should also placate Clang. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Dario Faggioli <dario.faggioli@citrix.com> Cc: Juergen Gross <jgross@suse.com> --- tools/libxl/libxl_numa.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)