[PATCH] libxl: only free vcpu info list when it is allocated

[PATCH] libxl: only free vcpu info list when it is allocated

[Wei Liu]

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

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


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

Re: [PATCH] libxl: only free vcpu info list when it is allocated

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1114 bytes --]

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
-- 
<<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 #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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

Re: [PATCH] libxl: only free vcpu info list when it is allocated

[Wei Liu]

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.

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