diff mbox

libxl: only free vcpu info list when it is allocated

Message ID 1477070247-32185-1-git-send-email-wei.liu2@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Wei Liu Oct. 21, 2016, 5:17 p.m. UTC
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(-)

Comments

Dario Faggioli Oct. 21, 2016, 5:26 p.m. UTC | #1
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
Wei Liu Oct. 24, 2016, 10:02 a.m. UTC | #2
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 mbox

Patch

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);