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

[George Dunlap]

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


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

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

[Dario Faggioli]

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
-- 
<> (Raistlin Majere)
-
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R Ltd., Cambridge (UK)From: Dario Faggioli 

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 
---
Cc: Ian Jackson 
Cc: Wei Liu 
Cc: George Dunlap 
---
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, );
-if (rc != 0) {
-LOGE(ERROR, "getting sched credit param");
-GC_FREE;
-return ERROR_FAIL;
+r = xc_sched_credit_params_get(ctx->xch, poolid, );
+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, );
-if ( rc < 0 ) {
-LOGE(ERROR, "setting sched credit param");
-GC_FREE;
-return ERROR_FAIL;
+r = xc_sched_credit_params_set(ctx->xch, poolid, );
+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,


signature.asc
Description: This is a digitally signed message part
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

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

[Ian Jackson]

Dario Faggioli writes ("[PATCH v2 08/10] libxl: fix coding style of credit1 
parameters related functions"):
> More specifically, the the error handling path is
> made compliant with libxl's codying style.
...
>  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 ?

Thanks,
Ian.

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

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

[Dario Faggioli]

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

No functional change intended.

Signed-off-by: Dario Faggioli 
---
Cc: Ian Jackson 
Cc: Wei Liu 
Cc: George Dunlap 
---
Changes from v1:
 * new patch, containing only the coding style changes from what was patch 14
   in v1.
---
 tools/libxl/libxl.c |   43 +++
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 997d94c..606d71a 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -5233,65 +5233,68 @@ 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, );
-if (rc != 0) {
-LOGE(ERROR, "getting sched credit param");
-GC_FREE;
-return ERROR_FAIL;
+r = xc_sched_credit_params_get(ctx->xch, poolid, );
+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, );
-if ( rc < 0 ) {
-LOGE(ERROR, "setting sched credit param");
-GC_FREE;
-return ERROR_FAIL;
+r = xc_sched_credit_params_set(ctx->xch, poolid, );
+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;
 
+ out:
 GC_FREE;
-return 0;
+return rc;
 }
 
 static int sched_credit2_domain_get(libxl__gc *gc, uint32_t domid,


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