diff mbox

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

Message ID 1475237087.5315.74.camel@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dario Faggioli Sept. 30, 2016, 12:04 p.m. UTC 
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

Comments

George Dunlap Sept. 30, 2016, 1:25 p.m. UTC | #1 
On 30/09/16 13:04, Dario Faggioli wrote:
> 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.

I've fixed it up in my tree, and added your Acked-by, Ian.

 -George
diff mbox

Patch

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,