[PATCH] tools/xl: Fix build error following c/s f52fbcf7

[PATCH] tools/xl: Fix build error following c/s f52fbcf7

[Andrew Cooper]

CentOS7 complains that 'ret' might be unused, and indeed this is the case for
`xl psr-hwinfo --cat`.

The logic for selecting which information to print was rather awkward.
Introduce a new 'all' which default to true, and is cleared if specific
options are selected.  This allows for a far more clear logic when choosing
whether to print information or not.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Chao Peng <chao.p.peng@linux.intel.com>

---
NB: Only compile tested.
---
 tools/libxl/xl_cmdimpl.c |   20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index eeb3b90..877165a 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -8426,8 +8426,8 @@ int main_psr_cat_show(int argc, char **argv)
 
 int main_psr_hwinfo(int argc, char **argv)
 {
-    int opt, ret;
-    int cmt = 0, cat = 0;
+    int opt, ret = 0;
+    bool all = true, cmt = false, cat = false;
     static struct option opts[] = {
         {"cmt", 0, 0, 'm'},
         {"cat", 0, 0, 'a'},
@@ -8437,25 +8437,17 @@ int main_psr_hwinfo(int argc, char **argv)
 
     SWITCH_FOREACH_OPT(opt, "ma", opts, "psr-hwinfo", 0) {
     case 'm':
-        cmt = 1;
+        all = false; cmt = true;
         break;
     case 'a':
-        cat = 1;
+        all = false; cat = true;
         break;
     }
 
-    if (!(cmt | cat)) {
-        cmt = 1;
-        cat = 1;
-    }
-
-    if (cmt)
+    if (!ret && (all || cmt))
         ret = psr_cmt_hwinfo();
 
-    if (ret)
-        return ret;
-
-    if (cat)
+    if (!ret && (all || cat))
         ret = psr_cat_hwinfo();
 
     return ret;
-- 
1.7.10.4

Re: [PATCH] tools/xl: Fix build error following c/s f52fbcf7

[Ian Campbell]

On Thu, 2015-07-09 at 11:36 +0100, Andrew Cooper wrote:
> CentOS7 complains that 'ret' might be unused, and indeed this is the case for
> `xl psr-hwinfo --cat`.
> 
> The logic for selecting which information to print was rather awkward.
> Introduce a new 'all' which default to true, and is cleared if specific
> options are selected.  This allows for a far more clear logic when choosing
> whether to print information or not.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Ian Campbell <Ian.Campbell@citrix.com>

I'll wait a bit before applying to give Chao a chance to have a look.

> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Chao Peng <chao.p.peng@linux.intel.com>
> 
> ---
> NB: Only compile tested.
> ---
>  tools/libxl/xl_cmdimpl.c |   20 ++++++--------------
>  1 file changed, 6 insertions(+), 14 deletions(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index eeb3b90..877165a 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -8426,8 +8426,8 @@ int main_psr_cat_show(int argc, char **argv)
>  
>  int main_psr_hwinfo(int argc, char **argv)
>  {
> -    int opt, ret;
> -    int cmt = 0, cat = 0;
> +    int opt, ret = 0;
> +    bool all = true, cmt = false, cat = false;
>      static struct option opts[] = {
>          {"cmt", 0, 0, 'm'},
>          {"cat", 0, 0, 'a'},
> @@ -8437,25 +8437,17 @@ int main_psr_hwinfo(int argc, char **argv)
>  
>      SWITCH_FOREACH_OPT(opt, "ma", opts, "psr-hwinfo", 0) {
>      case 'm':
> -        cmt = 1;
> +        all = false; cmt = true;
>          break;
>      case 'a':
> -        cat = 1;
> +        all = false; cat = true;
>          break;
>      }
>  
> -    if (!(cmt | cat)) {
> -        cmt = 1;
> -        cat = 1;
> -    }
> -
> -    if (cmt)
> +    if (!ret && (all || cmt))
>          ret = psr_cmt_hwinfo();
>  
> -    if (ret)
> -        return ret;
> -
> -    if (cat)
> +    if (!ret && (all || cat))
>          ret = psr_cat_hwinfo();
>  
>      return ret;

Re: [PATCH] tools/xl: Fix build error following c/s f52fbcf7

[Chao Peng]

On Thu, Jul 09, 2015 at 11:47:43AM +0100, Ian Campbell wrote:
> On Thu, 2015-07-09 at 11:36 +0100, Andrew Cooper wrote:
> > CentOS7 complains that 'ret' might be unused, and indeed this is the case for
> > `xl psr-hwinfo --cat`.
> > 
> > The logic for selecting which information to print was rather awkward.
> > Introduce a new 'all' which default to true, and is cleared if specific
> > options are selected.  This allows for a far more clear logic when choosing
> > whether to print information or not.
> > 
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
> 
> I'll wait a bit before applying to give Chao a chance to have a look.

Thanks and I verified it successfully on my psr-enabled box.

Chao
> 
> > CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> > CC: Wei Liu <wei.liu2@citrix.com>
> > CC: Chao Peng <chao.p.peng@linux.intel.com>
> > 

Re: [PATCH] tools/xl: Fix build error following c/s f52fbcf7

[Ian Campbell]

On Thu, 2015-07-09 at 21:31 +0800, Chao Peng wrote:
> On Thu, Jul 09, 2015 at 11:47:43AM +0100, Ian Campbell wrote:
> > On Thu, 2015-07-09 at 11:36 +0100, Andrew Cooper wrote:
> > > CentOS7 complains that 'ret' might be unused, and indeed this is the case for
> > > `xl psr-hwinfo --cat`.
> > > 
> > > The logic for selecting which information to print was rather awkward.
> > > Introduce a new 'all' which default to true, and is cleared if specific
> > > options are selected.  This allows for a far more clear logic when choosing
> > > whether to print information or not.
> > > 
> > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > 
> > Acked-by: Ian Campbell <Ian.Campbell@citrix.com>
> > 
> > I'll wait a bit before applying to give Chao a chance to have a look.
> 
> Thanks and I verified it successfully on my psr-enabled box.

Thanks. I converted this into a

        Tested-by: Chao Peng <chao.p.peng@linux.intel.com>

and have applied.