Re: [PATCH 11/25] staging: lustre: libcfs: invert error handling for cfs_cpt_table_print

[Dan Carpenter]

On Mon, Apr 16, 2018 at 12:09:53AM -0400, James Simmons wrote:
> From: Amir Shehata 
> 
> Instead of setting rc to -EFBIG for several cases in the loop lets
> initialize rc to -EFBIG and just break out of the loop in case of
> failure. Just set rc to zero once we successfully finish the loop.
> 
> Signed-off-by: Amir Shehata 
> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-7734
> Reviewed-on: http://review.whamcloud.com/18916
> Reviewed-by: Olaf Weber 
> Reviewed-by: Doug Oucharek 
> Signed-off-by: James Simmons 
> ---
>  .../staging/lustre/lnet/libcfs/linux/linux-cpu.c   | 22 
> ++
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c 
> b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
> index bbf89b8..6d8dcd3 100644
> --- a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
> +++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
> @@ -158,29 +158,26 @@ struct cfs_cpt_table *
>  cfs_cpt_table_print(struct cfs_cpt_table *cptab, char *buf, int len)
>  {
>   char *tmp = buf;
> - int rc = 0;
> + int rc = -EFBIG;
>   int i;
>   int j;
>  
>   for (i = 0; i < cptab->ctb_nparts; i++) {
> - if (len > 0) {
> - rc = snprintf(tmp, len, "%d\t:", i);
> - len -= rc;
> - }
> + if (len <= 0)
> + goto out;
> +
> + rc = snprintf(tmp, len, "%d\t:", i);
> + len -= rc;
>  
> - if (len <= 0) {
> - rc = -EFBIG;
> + if (len <= 0)
>   goto out;

This patch says it's a cleanup but the only thing it does is introduce
"forgot to set the error" bugs.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 11/25] staging: lustre: libcfs: invert error handling for cfs_cpt_table_print

[James Simmons]

From: Amir Shehata 

Instead of setting rc to -EFBIG for several cases in the loop lets
initialize rc to -EFBIG and just break out of the loop in case of
failure. Just set rc to zero once we successfully finish the loop.

Signed-off-by: Amir Shehata 
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-7734
Reviewed-on: http://review.whamcloud.com/18916
Reviewed-by: Olaf Weber 
Reviewed-by: Doug Oucharek 
Signed-off-by: James Simmons 
---
 .../staging/lustre/lnet/libcfs/linux/linux-cpu.c   | 22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c 
b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
index bbf89b8..6d8dcd3 100644
--- a/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
+++ b/drivers/staging/lustre/lnet/libcfs/linux/linux-cpu.c
@@ -158,29 +158,26 @@ struct cfs_cpt_table *
 cfs_cpt_table_print(struct cfs_cpt_table *cptab, char *buf, int len)
 {
char *tmp = buf;
-   int rc = 0;
+   int rc = -EFBIG;
int i;
int j;
 
for (i = 0; i < cptab->ctb_nparts; i++) {
-   if (len > 0) {
-   rc = snprintf(tmp, len, "%d\t:", i);
-   len -= rc;
-   }
+   if (len <= 0)
+   goto out;
+
+   rc = snprintf(tmp, len, "%d\t:", i);
+   len -= rc;
 
-   if (len <= 0) {
-   rc = -EFBIG;
+   if (len <= 0)
goto out;
-   }
 
tmp += rc;
for_each_cpu(j, cptab->ctb_parts[i].cpt_cpumask) {
-   rc = snprintf(tmp, len, "%d ", j);
+   rc = snprintf(tmp, len, " %d", j);
len -= rc;
-   if (len <= 0) {
-   rc = -EFBIG;
+   if (len <= 0)
goto out;
-   }
tmp += rc;
}
 
@@ -189,6 +186,7 @@ struct cfs_cpt_table *
len--;
}
 
+   rc = 0;
  out:
if (rc < 0)
return rc;
-- 
1.8.3.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel