lkml.org 
[lkml]   [2018]   [Apr]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 08/25] staging: lustre: libcfs: add cpu distance handling
On Mon, Apr 16, 2018 at 12:09:50AM -0400, James Simmons wrote:
> +int cfs_cpt_distance_print(struct cfs_cpt_table *cptab, char *buf, int len)
> +{
> + char *tmp = buf;
> + int rc = -EFBIG;
> + int i;
> + int j;
> +
> + for (i = 0; i < cptab->ctb_nparts; i++) {
> + if (len <= 0)
> + goto err;
> +
> + rc = snprintf(tmp, len, "%d\t:", i);
> + len -= rc;
> +
> + if (len <= 0)
> + goto err;


The "goto err" here is sort of an example of a "do-nothing" goto. There
are no resources to free or anything like that.

I don't like do-nothing gotos because "return -EFBIG;" is self
documenting code and "goto err;" is totally ambiguous what it does. The
second problem is that do-nothing error labels easily turn into
do-everything one err style error paths which I loath. And the third
problem is that they introduce "forgot to set the error code" bugs.

It looks like we forgot to set the error code here although it's also
possible that was intended. This code is ambiguous.

> +
> + tmp += rc;
> + for (j = 0; j < cptab->ctb_nparts; j++) {
> + rc = snprintf(tmp, len, " %d:%d",
> + j, cptab->ctb_parts[i].cpt_distance[j]);
> + len -= rc;
> + if (len <= 0)
> + goto err;
> + tmp += rc;
> + }
> +
> + *tmp = '\n';
> + tmp++;
> + len--;
> + }
> + rc = 0;
> +err:
> + if (rc < 0)
> + return rc;
> +
> + return tmp - buf;
> +}

regards,
dan carpenter

\
 
 \ /
  Last update: 2018-04-16 16:46    [W:0.129 / U:0.104 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site