diff mbox

[10/10] libxl: libxl_tmem functions improving coding style

Message ID 1459514413-18682-11-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 
In accordance with CODING_SYTLE:
 - Use 'r' for return values to functions whose return values are a
   different error space (like xc_tmem_control, xc_tmem_auth)

Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>
---
 tools/libxl/libxl.c | 58 ++++++++++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 29 deletions(-)

Comments

Konrad Rzeszutek Wilk April 1, 2016, 3:14 p.m. UTC | #1 
On Fri, Apr 01, 2016 at 02:40:13PM +0200, Paulina Szubarczyk wrote:
> In accordance with CODING_SYTLE:
>  - Use 'r' for return values to functions whose return values are a
>    different error space (like xc_tmem_control, xc_tmem_auth)
> 
> Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Roger Pau Monne April 1, 2016, 3:39 p.m. UTC | #2 
On Fri, 1 Apr 2016, Paulina Szubarczyk wrote:

> In accordance with CODING_SYTLE:
>  - Use 'r' for return values to functions whose return values are a
>    different error space (like xc_tmem_control, xc_tmem_auth)
> 
> Signed-off-by: Paulina Szubarczyk <paulinaszubarczyk@gmail.com>
> ---
>  tools/libxl/libxl.c | 58 ++++++++++++++++++++++++++---------------------------
>  1 file changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 6bb2f82..5794ade 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -6107,14 +6107,14 @@ uint32_t libxl_vm_get_start_time(libxl_ctx *ctx, uint32_t domid)
>  
>  char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long)
>  {
> -    int rc;
> +    int r;
>      char _buf[32768];
>      GC_INIT(ctx);
>  
> -    rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_LIST, domid, 32768, use_long,
> +    r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_LIST, domid, 32768, use_long,
>                           _buf);
> -    if (rc < 0) {
> -        LOGEV(ERROR, rc, "Can not get tmem list");
> +    if (r < 0) {
> +        LOGEV(ERROR, r, "Can not get tmem list");
>          GC_FREE;
>          return NULL;
>      }
> @@ -6125,36 +6125,36 @@ char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long)
>  
>  int libxl_tmem_freeze(libxl_ctx *ctx, uint32_t domid)
>  {
> -    int rc;
> +    int r;
>      GC_INIT(ctx);
>  
> -    rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_FREEZE, domid, 0, 0,
> +    r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_FREEZE, domid, 0, 0,
>                           NULL);
> -    if (rc < 0) {
> -        LOGEV(ERROR, rc, "Can not freeze tmem pools");
> +    if (r < 0) {
> +        LOGEV(ERROR, r, "Can not freeze tmem pools");
>          GC_FREE;
>          return ERROR_FAIL;
>      }
>  
>      GC_FREE;
> -    return rc;
> +    return r;

This is conceptually wrong IMHO, you are returning an error code that's 
not in libxl space from a libxl function (although due to the "return" 
above this is always going to be 0). I would just return 0 here, or add an 
"out" label, keep rc and set it in the error case above replacing the 
return with a goto out.

Roger.
diff mbox

Patch

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 6bb2f82..5794ade 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -6107,14 +6107,14 @@  uint32_t libxl_vm_get_start_time(libxl_ctx *ctx, uint32_t domid)
 
 char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long)
 {
-    int rc;
+    int r;
     char _buf[32768];
     GC_INIT(ctx);
 
-    rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_LIST, domid, 32768, use_long,
+    r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_LIST, domid, 32768, use_long,
                          _buf);
-    if (rc < 0) {
-        LOGEV(ERROR, rc, "Can not get tmem list");
+    if (r < 0) {
+        LOGEV(ERROR, r, "Can not get tmem list");
         GC_FREE;
         return NULL;
     }
@@ -6125,36 +6125,36 @@  char *libxl_tmem_list(libxl_ctx *ctx, uint32_t domid, int use_long)
 
 int libxl_tmem_freeze(libxl_ctx *ctx, uint32_t domid)
 {
-    int rc;
+    int r;
     GC_INIT(ctx);
 
-    rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_FREEZE, domid, 0, 0,
+    r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_FREEZE, domid, 0, 0,
                          NULL);
-    if (rc < 0) {
-        LOGEV(ERROR, rc, "Can not freeze tmem pools");
+    if (r < 0) {
+        LOGEV(ERROR, r, "Can not freeze tmem pools");
         GC_FREE;
         return ERROR_FAIL;
     }
 
     GC_FREE;
-    return rc;
+    return r;
 }
 
 int libxl_tmem_thaw(libxl_ctx *ctx, uint32_t domid)
 {
-    int rc;
+    int r;
     GC_INIT(ctx);
 
-    rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_THAW, domid, 0, 0,
+    r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_THAW, domid, 0, 0,
                          NULL);
-    if (rc < 0) {
-        LOGEV(ERROR, rc, "Can not thaw tmem pools");
+    if (r < 0) {
+        LOGEV(ERROR, r, "Can not thaw tmem pools");
         GC_FREE;
         return ERROR_FAIL;
     }
 
     GC_FREE;
-    return rc;
+    return r;
 }
 
 static int32_t tmem_setop_from_string(char *set_name)
@@ -6171,7 +6171,7 @@  static int32_t tmem_setop_from_string(char *set_name)
 
 int libxl_tmem_set(libxl_ctx *ctx, uint32_t domid, char* name, uint32_t set)
 {
-    int rc;
+    int r;
     int32_t subop = tmem_setop_from_string(name);
     GC_INIT(ctx);
 
@@ -6180,48 +6180,48 @@  int libxl_tmem_set(libxl_ctx *ctx, uint32_t domid, char* name, uint32_t set)
         GC_FREE;
         return ERROR_INVAL;
     }
-    rc = xc_tmem_control(ctx->xch, -1, subop, domid, set, 0, NULL);
-    if (rc < 0) {
-        LOGEV(ERROR, rc, "Can not set tmem %s", name);
+    r = xc_tmem_control(ctx->xch, -1, subop, domid, set, 0, NULL);
+    if (r < 0) {
+        LOGEV(ERROR, r, "Can not set tmem %s", name);
         GC_FREE;
         return ERROR_FAIL;
     }
 
     GC_FREE;
-    return rc;
+    return r;
 }
 
 int libxl_tmem_shared_auth(libxl_ctx *ctx, uint32_t domid,
                            char* uuid, int auth)
 {
-    int rc;
+    int r;
     GC_INIT(ctx);
 
-    rc = xc_tmem_auth(ctx->xch, domid, uuid, auth);
-    if (rc < 0) {
-        LOGEV(ERROR, rc, "Can not set tmem shared auth");
+    r = xc_tmem_auth(ctx->xch, domid, uuid, auth);
+    if (r < 0) {
+        LOGEV(ERROR, r, "Can not set tmem shared auth");
         GC_FREE;
         return ERROR_FAIL;
     }
 
     GC_FREE;
-    return rc;
+    return r;
 }
 
 int libxl_tmem_freeable(libxl_ctx *ctx)
 {
-    int rc;
+    int r;
     GC_INIT(ctx);
 
-    rc = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB, -1, 0, 0, 0);
-    if (rc < 0) {
-        LOGEV(ERROR, rc, "Can not get tmem freeable memory");
+    r = xc_tmem_control(ctx->xch, -1, XEN_SYSCTL_TMEM_OP_QUERY_FREEABLE_MB, -1, 0, 0, 0);
+    if (r < 0) {
+        LOGEV(ERROR, r, "Can not get tmem freeable memory");
         GC_FREE;
         return ERROR_FAIL;
     }
 
     GC_FREE;
-    return rc;
+    return r;
 }
 
 int libxl_get_freecpus(libxl_ctx *ctx, libxl_bitmap *cpumap)