Re: [PATCH v2 08/10] libxl: fix coding style of credit1 parameters related functions

[Dario Faggioli <dario.faggioli@citrix.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 1364 bytes --]

On Fri, 2016-09-30 at 11:24 +0100, Ian Jackson wrote:
> Dario Faggioli writes ("[PATCH v2 08/10] libxl: fix coding style of
> credit1 parameters related functions"):
> >  int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
> >                                    libxl_sched_credit_params
> > *scinfo)
> >  {
> >      struct xen_sysctl_credit_schedule sparam;
> > -    int rc=0;
> > +    int r, rc;
> ...
> > 
> >  
> >      scinfo->tslice_ms = sparam.tslice_ms;
> >      scinfo->ratelimit_us = sparam.ratelimit_us;
> >  
> > + out:
> >      GC_FREE;
> > -    return 0;
> > +    return rc;
> 
> I think this is missing an assignment
> 
>   rc = 0;
> 
> on the successful exit path, just before out.  Am I wrong ?
> 
Indeed it's missing. It was not necessary in v1 of this patch, so I
must have failed to notice that it was, when splitting that in two.

Sorry.

Not sure how to proceed, so I'm attaching an updated version of the
patch to this email.

Thanks and Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.1.2: libxl-fix-coding-style-sched-param.patch --]
[-- Type: text/x-patch, Size: 3487 bytes --]

From: Dario Faggioli <dario.faggioli@citrix.com>

libxl: fix coding style of credit1 parameters related functions

More specifically, the the error handling path is
made compliant with libxl's codying style.

No functional change intended.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
---
Changes from v2:
 * add a missing 'rc = 0', as noted during review.

Changes from v1:
 * new patch, containing only the coding style changes from what was patch 14
   in v1.

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index a46b827..d2552f9 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5260,65 +5260,69 @@ int libxl_sched_credit_params_get(libxl_ctx *ctx, uint32_t poolid,
                                   libxl_sched_credit_params *scinfo)
 {
     struct xen_sysctl_credit_schedule sparam;
-    int rc;
+    int r, rc;
     GC_INIT(ctx);
 
-    rc = xc_sched_credit_params_get(ctx->xch, poolid, &sparam);
-    if (rc != 0) {
-        LOGE(ERROR, "getting sched credit param");
-        GC_FREE;
-        return ERROR_FAIL;
+    r = xc_sched_credit_params_get(ctx->xch, poolid, &sparam);
+    if (r < 0) {
+        LOGE(ERROR, "getting Credit scheduler parameters");
+        rc = ERROR_FAIL;
+        goto out;
     }
 
     scinfo->tslice_ms = sparam.tslice_ms;
     scinfo->ratelimit_us = sparam.ratelimit_us;
 
+    rc = 0;
+ out:
     GC_FREE;
-    return 0;
+    return rc;
 }
 
 int libxl_sched_credit_params_set(libxl_ctx *ctx, uint32_t poolid,
                                   libxl_sched_credit_params *scinfo)
 {
     struct xen_sysctl_credit_schedule sparam;
-    int rc=0;
+    int r, rc;
     GC_INIT(ctx);
 
     if (scinfo->tslice_ms <  XEN_SYSCTL_CSCHED_TSLICE_MIN
         || scinfo->tslice_ms > XEN_SYSCTL_CSCHED_TSLICE_MAX) {
         LOG(ERROR, "Time slice out of range, valid range is from %d to %d",
             XEN_SYSCTL_CSCHED_TSLICE_MIN, XEN_SYSCTL_CSCHED_TSLICE_MAX);
-        GC_FREE;
-        return ERROR_INVAL;
+        rc = ERROR_INVAL;
+        goto out;
     }
     if (scinfo->ratelimit_us <  XEN_SYSCTL_SCHED_RATELIMIT_MIN
         || scinfo->ratelimit_us > XEN_SYSCTL_SCHED_RATELIMIT_MAX) {
         LOG(ERROR, "Ratelimit out of range, valid range is from %d to %d",
             XEN_SYSCTL_SCHED_RATELIMIT_MIN, XEN_SYSCTL_SCHED_RATELIMIT_MAX);
-        GC_FREE;
-        return ERROR_INVAL;
+        rc = ERROR_INVAL;
+        goto out;
     }
     if (scinfo->ratelimit_us > scinfo->tslice_ms*1000) {
         LOG(ERROR, "Ratelimit cannot be greater than timeslice");
-        GC_FREE;
-        return ERROR_INVAL;
+        rc = ERROR_INVAL;
+        goto out;
     }
 
     sparam.tslice_ms = scinfo->tslice_ms;
     sparam.ratelimit_us = scinfo->ratelimit_us;
 
-    rc = xc_sched_credit_params_set(ctx->xch, poolid, &sparam);
-    if ( rc < 0 ) {
-        LOGE(ERROR, "setting sched credit param");
-        GC_FREE;
-        return ERROR_FAIL;
+    r = xc_sched_credit_params_set(ctx->xch, poolid, &sparam);
+    if ( r < 0 ) {
+        LOGE(ERROR, "Setting Credit scheduler parameters");
+        rc = ERROR_FAIL;
+        goto out;
     }
 
     scinfo->tslice_ms = sparam.tslice_ms;
     scinfo->ratelimit_us = sparam.ratelimit_us;
 
+    rc = 0;
+ out:
     GC_FREE;
-    return 0;
+    return rc;
 }
 
 static int sched_credit2_domain_get(libxl__gc *gc, uint32_t domid,

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

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