Re: [Xen-devel] [PATCH 08/10] libxl: improve main_tmem_* return codes

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Fri, Apr 01, 2016 at 02:40:11PM +0200, Paulina Szubarczyk wrote:
> Functions libxl_tmem_freeze(), libxl_tmem_thaw(), libxl_tmem_set() and
> libxl_tmem_shared_auth() located in libxl.c file return 
> ERROR_FAIL/ERROR_INVAL 
> or internal error codes from libxc library.
> 
> Improve main_tmem_* return codes by returning EXIT_{SUCCESS/FAILURE}
> accordingly to return codes of those functions.
> 
> Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@xxxxxxxxx>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, albeit

The CODING_STYLE says:
Every indented statement is braced apart from blocks that contain just
one statement."

Which is not very clear. I think it is missing an ',', which would make it

Every indented statement is braced, apart from blocks that contain just one 
statement.

Which means that:

> ---
>  tools/libxl/xl_cmdimpl.c | 51 
> ++++++++++++++++++++++++++++++------------------
>  1 file changed, 32 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 38129f4..52c3b9b 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -7519,7 +7519,7 @@ int main_tmem_freeze(int argc, char **argv)
>      if (!dom && all == 0) {
>          fprintf(stderr, "You must specify -a or a domain id.\n\n");
>          help("tmem-freeze");
> -        return 1;
> +        return EXIT_FAILURE;
>      }
>  
>      if (all)
> @@ -7527,8 +7527,11 @@ int main_tmem_freeze(int argc, char **argv)
>      else
>          domid = find_domain(dom);
>  
> -    libxl_tmem_freeze(ctx, domid);
> -    return 0;
> +    if (libxl_tmem_freeze(ctx, domid) < 0) {
> +        return EXIT_FAILURE;
> +    }

This and the other three can drop the braces.

> +
> +    return EXIT_SUCCESS;
>  }
>  
>  int main_tmem_thaw(int argc, char **argv)
> @@ -7548,7 +7551,7 @@ int main_tmem_thaw(int argc, char **argv)
>      if (!dom && all == 0) {
>          fprintf(stderr, "You must specify -a or a domain id.\n\n");
>          help("tmem-thaw");
> -        return 1;
> +        return EXIT_FAILURE;
>      }
>  
>      if (all)
> @@ -7556,8 +7559,11 @@ int main_tmem_thaw(int argc, char **argv)
>      else
>          domid = find_domain(dom);
>  
> -    libxl_tmem_thaw(ctx, domid);
> -    return 0;
> +    if (libxl_tmem_thaw(ctx, domid) < 0) {
> +        return EXIT_FAILURE;
> +    }
> +
> +    return EXIT_SUCCESS;
>  }
>  
>  int main_tmem_set(int argc, char **argv)
> @@ -7568,6 +7574,7 @@ int main_tmem_set(int argc, char **argv)
>      int opt_w = 0, opt_c = 0, opt_p = 0;
>      int all = 0;
>      int opt;
> +    int r = 0;
>  
>      SWITCH_FOREACH_OPT(opt, "aw:c:p:", NULL, "tmem-set", 0) {
>      case 'a':
> @@ -7591,7 +7598,7 @@ int main_tmem_set(int argc, char **argv)
>      if (!dom && all == 0) {
>          fprintf(stderr, "You must specify -a or a domain id.\n\n");
>          help("tmem-set");
> -        return 1;
> +        return EXIT_FAILURE;
>      }
>  
>      if (all)
> @@ -7602,17 +7609,21 @@ int main_tmem_set(int argc, char **argv)
>      if (!opt_w && !opt_c && !opt_p) {
>          fprintf(stderr, "No set value specified.\n\n");
>          help("tmem-set");
> -        return 1;
> +        return EXIT_FAILURE;
>      }
>  
>      if (opt_w)
> -        libxl_tmem_set(ctx, domid, "weight", weight);
> +        r = libxl_tmem_set(ctx, domid, "weight", weight);
>      if (opt_c)
> -        libxl_tmem_set(ctx, domid, "cap", cap);
> +        r = libxl_tmem_set(ctx, domid, "cap", cap);
>      if (opt_p)
> -        libxl_tmem_set(ctx, domid, "compress", compress);
> +        r = libxl_tmem_set(ctx, domid, "compress", compress);
>  
> -    return 0;
> +    if (r < 0) {
> +        return EXIT_FAILURE;
> +    }
> +
> +    return EXIT_SUCCESS;
>  }
>  
>  int main_tmem_shared_auth(int argc, char **argv)
> @@ -7642,7 +7653,7 @@ int main_tmem_shared_auth(int argc, char **argv)
>      if (!dom && all == 0) {
>          fprintf(stderr, "You must specify -a or a domain id.\n\n");
>          help("tmem-shared-auth");
> -        return 1;
> +        return EXIT_FAILURE;
>      }
>  
>      if (all)
> @@ -7653,18 +7664,20 @@ int main_tmem_shared_auth(int argc, char **argv)
>      if (uuid == NULL || autharg == NULL) {
>          fprintf(stderr, "No uuid or auth specified.\n\n");
>          help("tmem-shared-auth");
> -        return 1;
> +        return EXIT_FAILURE;
>      }
>  
>      auth = strtol(autharg, &endptr, 10);
>      if (*endptr != '\0') {
>          fprintf(stderr, "Invalid auth, valid auth are <0|1>.\n\n");
> -        return 1;
> +        return EXIT_FAILURE;
>      }
>  
> -    libxl_tmem_shared_auth(ctx, domid, uuid, auth);
> +    if (libxl_tmem_shared_auth(ctx, domid, uuid, auth) < 0) {
> +        return EXIT_FAILURE;
> +    }
>  
> -    return 0;
> +    return EXIT_SUCCESS;
>  }
>  
>  int main_tmem_freeable(int argc, char **argv)
> @@ -7678,10 +7691,10 @@ int main_tmem_freeable(int argc, char **argv)
>  
>      mb = libxl_tmem_freeable(ctx);
>      if (mb == -1)
> -        return -1;
> +        return EXIT_FAILURE;
>  
>      printf("%d\n", mb);
> -    return 0;
> +    return EXIT_SUCCESS;
>  }
>  
>  int main_cpupoolcreate(int argc, char **argv)
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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