[PATCH] xl: don't print out uninitialised rc

[PATCH] xl: don't print out uninitialised rc

[Wei Liu]

In 5b725e56 (xl: improve return and exit codes of vcpu related
functions), the return value of libxl_cpu_bitmap_alloc was not stored in
rc anymore. Yet the subsequent fprintf still used that.

While it is trivial to reinstate the original implementation, xl's
caller has no idea what a libxl error code is so printing out rc won't
be that useful. Don't print out rc in that case.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Dario Faggioli <dario.faggioli@citrix.com>
Cc: Harmandeep Kaur <write.harmandeep@gmail.com>
---
 tools/libxl/xl_cmdimpl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 2756d2f..6056344 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -5474,7 +5474,7 @@ static int vcpuset(uint32_t domid, const char* nr_vcpus, int check_host)
             return 1;
     }
     if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) {
-        fprintf(stderr, "libxl_cpu_bitmap_alloc failed, rc: %d\n", rc);
+        fprintf(stderr, "libxl_cpu_bitmap_alloc failed\n");
         return 1;
     }
     for (i = 0; i < max_vcpus; i++)
-- 
2.1.4

Re: [PATCH] xl: don't print out uninitialised rc

[Ian Campbell]

On Wed, 2015-11-04 at 11:15 +0000, Wei Liu wrote:
> In 5b725e56 (xl: improve return and exit codes of vcpu related
> functions), the return value of libxl_cpu_bitmap_alloc was not stored in
> rc anymore. Yet the subsequent fprintf still used that.
> 
> While it is trivial to reinstate the original implementation, xl's
> caller has no idea what a libxl error code is so printing out rc won't
> be that useful.

I don't buy this I'm afraid. xl prints the libxl error code in lots of
places and it is indeed useful for a person (e.g. a developer reading a bug
report) since they can go and look the number up.

I'm not sure where "xl's caller" comes into it, since this is only about
printing and not returning.

>  Don't print out rc in that case.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Dario Faggioli <dario.faggioli@citrix.com>
> Cc: Harmandeep Kaur <write.harmandeep@gmail.com>
> ---
>  tools/libxl/xl_cmdimpl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 2756d2f..6056344 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -5474,7 +5474,7 @@ static int vcpuset(uint32_t domid, const char*
> nr_vcpus, int check_host)
>              return 1;
>      }
>      if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) {
> -        fprintf(stderr, "libxl_cpu_bitmap_alloc failed, rc: %d\n", rc);
> +        fprintf(stderr, "libxl_cpu_bitmap_alloc failed\n");
>          return 1;
>      }
>      for (i = 0; i < max_vcpus; i++)

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

Re: [PATCH] xl: don't print out uninitialised rc

[Wei Liu]

On Wed, Nov 04, 2015 at 11:23:29AM +0000, Ian Campbell wrote:
> On Wed, 2015-11-04 at 11:15 +0000, Wei Liu wrote:
> > In 5b725e56 (xl: improve return and exit codes of vcpu related
> > functions), the return value of libxl_cpu_bitmap_alloc was not stored in
> > rc anymore. Yet the subsequent fprintf still used that.
> > 
> > While it is trivial to reinstate the original implementation, xl's
> > caller has no idea what a libxl error code is so printing out rc won't
> > be that useful.
> 
> I don't buy this I'm afraid. xl prints the libxl error code in lots of
> places and it is indeed useful for a person (e.g. a developer reading a bug
> report) since they can go and look the number up.
> 
> I'm not sure where "xl's caller" comes into it, since this is only about
> printing and not returning.
> 

So the original implementation it is -- I don't feel strong enough to
argue for one way or another.

Wei.

> >  Don't print out rc in that case.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > Cc: Dario Faggioli <dario.faggioli@citrix.com>
> > Cc: Harmandeep Kaur <write.harmandeep@gmail.com>
> > ---
> >  tools/libxl/xl_cmdimpl.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > index 2756d2f..6056344 100644
> > --- a/tools/libxl/xl_cmdimpl.c
> > +++ b/tools/libxl/xl_cmdimpl.c
> > @@ -5474,7 +5474,7 @@ static int vcpuset(uint32_t domid, const char*
> > nr_vcpus, int check_host)
> >              return 1;
> >      }
> >      if (libxl_cpu_bitmap_alloc(ctx, &cpumap, max_vcpus)) {
> > -        fprintf(stderr, "libxl_cpu_bitmap_alloc failed, rc: %d\n", rc);
> > +        fprintf(stderr, "libxl_cpu_bitmap_alloc failed\n");
> >          return 1;
> >      }
> >      for (i = 0; i < max_vcpus; i++)