diff mbox

[08/10] libxl: improve main_tmem_* return codes

Message ID 1459514413-18682-9-git-send-email-paulinaszubarczyk@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paulina Szubarczyk April 1, 2016, 12:40 p.m. UTC 
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@gmail.com>
---
 tools/libxl/xl_cmdimpl.c | 51 ++++++++++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 19 deletions(-)

Comments

Roger Pau Monné April 1, 2016, 2:50 p.m. UTC | #1 
On Fri, 1 Apr 2016, 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@gmail.com>

LGTM:

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Roger.
Konrad Rzeszutek Wilk April 1, 2016, 3:09 p.m. UTC | #2 
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@gmail.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, 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@lists.xen.org
> http://lists.xen.org/xen-devel
diff mbox

Patch

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