Re: [lustre-devel] [PATCH] staging: lustre: split error handling code into multiple labels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> Instead of using a switch-case statement to find out what kind of error
> has just happened, split error handling logic into multiple labels and
> jump right into the appropriate label to do the error handling. This way
> it is easier to follow different code paths. It also looks easy on the
> eyes.
> 
> Additionally silences the following coccinelle warning:
> 
> drivers/staging/lustre/lustre/obdecho/echo_client.c:762:22-27: ERROR: ed
> is NULL but dereferenced.
> 
> Signed-off-by: Cihangir Akturk <cakturk@xxxxxxxxx>

Acked-by: James Simmons <jsimmons@xxxxxxxxxxxxx>

I also tested it and saw no regressions.

> ---
>  .../staging/lustre/lustre/obdecho/echo_client.c    | 54 ++++++++--------------
>  1 file changed, 20 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/staging/lustre/lustre/obdecho/echo_client.c b/drivers/staging/lustre/lustre/obdecho/echo_client.c
> index a752bb4..f143f7a 100644
> --- a/drivers/staging/lustre/lustre/obdecho/echo_client.c
> +++ b/drivers/staging/lustre/lustre/obdecho/echo_client.c
> @@ -668,8 +668,7 @@ static struct lu_device *echo_device_alloc(const struct lu_env *env,
>  	struct obd_device  *obd = NULL; /* to keep compiler happy */
>  	struct obd_device  *tgt;
>  	const char *tgt_type_name;
> -	int rc;
> -	int cleanup = 0;
> +	int rc, err;
>  
>  	ed = kzalloc(sizeof(*ed), GFP_NOFS);
>  	if (!ed) {
> @@ -677,16 +676,14 @@ static struct lu_device *echo_device_alloc(const struct lu_env *env,
>  		goto out;
>  	}
>  
> -	cleanup = 1;
>  	cd = &ed->ed_cl;
>  	rc = cl_device_init(cd, t);
>  	if (rc)
> -		goto out;
> +		goto out_free;
>  
>  	cd->cd_lu_dev.ld_ops = &echo_device_lu_ops;
>  	cd->cd_ops = &echo_device_cl_ops;
>  
> -	cleanup = 2;
>  	obd = class_name2obd(lustre_cfg_string(cfg, 0));
>  	LASSERT(obd);
>  	LASSERT(env);
> @@ -696,28 +693,25 @@ static struct lu_device *echo_device_alloc(const struct lu_env *env,
>  		CERROR("Can not find tgt device %s\n",
>  		       lustre_cfg_string(cfg, 1));
>  		rc = -ENODEV;
> -		goto out;
> +		goto out_device_fini;
>  	}
>  
>  	next = tgt->obd_lu_dev;
>  	if (!strcmp(tgt->obd_type->typ_name, LUSTRE_MDT_NAME)) {
>  		CERROR("echo MDT client must be run on server\n");
>  		rc = -EOPNOTSUPP;
> -		goto out;
> +		goto out_device_fini;
>  	}
>  
>  	rc = echo_site_init(env, ed);
>  	if (rc)
> -		goto out;
> -
> -	cleanup = 3;
> +		goto out_device_fini;
>  
>  	rc = echo_client_setup(env, obd, cfg);
>  	if (rc)
> -		goto out;
> +		goto out_site_fini;
>  
>  	ed->ed_ec = &obd->u.echo_client;
> -	cleanup = 4;
>  
>  	/* if echo client is to be stacked upon ost device, the next is
>  	 * NULL since ost is not a clio device so far
> @@ -729,7 +723,7 @@ static struct lu_device *echo_device_alloc(const struct lu_env *env,
>  	if (next) {
>  		if (next->ld_site) {
>  			rc = -EBUSY;
> -			goto out;
> +			goto out_cleanup;
>  		}
>  
>  		next->ld_site = &ed->ed_site->cs_lu;
> @@ -737,7 +731,7 @@ static struct lu_device *echo_device_alloc(const struct lu_env *env,
>  						next->ld_type->ldt_name,
>  							      NULL);
>  		if (rc)
> -			goto out;
> +			goto out_cleanup;
>  
>  	} else {
>  		LASSERT(strcmp(tgt_type_name, LUSTRE_OST_NAME) == 0);
> @@ -745,27 +739,19 @@ static struct lu_device *echo_device_alloc(const struct lu_env *env,
>  
>  	ed->ed_next = next;
>  	return &cd->cd_lu_dev;
> -out:
> -	switch (cleanup) {
> -	case 4: {
> -		int rc2;
> -
> -		rc2 = echo_client_cleanup(obd);
> -		if (rc2)
> -			CERROR("Cleanup obd device %s error(%d)\n",
> -			       obd->obd_name, rc2);
> -	}
>  
> -	case 3:
> -		echo_site_fini(env, ed);
> -	case 2:
> -		cl_device_fini(&ed->ed_cl);
> -	case 1:
> -		kfree(ed);
> -	case 0:
> -	default:
> -		break;
> -	}
> +out_cleanup:
> +	err = echo_client_cleanup(obd);
> +	if (err)
> +		CERROR("Cleanup obd device %s error(%d)\n",
> +		       obd->obd_name, err);
> +out_site_fini:
> +	echo_site_fini(env, ed);
> +out_device_fini:
> +	cl_device_fini(&ed->ed_cl);
> +out_free:
> +	kfree(ed);
> +out:
>  	return ERR_PTR(rc);
>  }
>  
> -- 
> 2.1.4
> 
> _______________________________________________
> lustre-devel mailing list
> lustre-devel@xxxxxxxxxxxxxxxx
> http://lists.lustre.org/listinfo.cgi/lustre-devel-lustre.org
> 
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux