Re: [PATCH 9/9] TTY: call tty_port_destroy in the rest of drivers

[Tilman Schmidt <tilman@imap.cc>]

[-- Attachment #1: Type: text/plain, Size: 2433 bytes --]

Hi Jiri,

Two remarks wrt the Gigaset driver.

Am 15.11.2012 09:49, schrieb Jiri Slaby:
> diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
> index 30a6b17..bc9d89a 100644
> --- a/drivers/isdn/gigaset/common.c
> +++ b/drivers/isdn/gigaset/common.c
> @@ -518,6 +518,7 @@ f_bcs:	gig_dbg(DEBUG_INIT, "freeing bcs[]");
>  	kfree(cs->bcs);
>  f_cs:	gig_dbg(DEBUG_INIT, "freeing cs");
>  	mutex_unlock(&cs->mutex);
> +	tty_port_destroy(&cs->port);
>  	free_cs(cs);
>  }
>  EXPORT_SYMBOL_GPL(gigaset_freecs);

It is not ok to call tty_port_destroy() unconditionally here.
gigaset_freecs() may be called from gigaset_initcs() before
the tty_port_init(&cs->port) call if a memory allocation fails.
This is best fixed by moving that call to case 1 of the preceding
switch statement because cs_init >= 1 covers exactly the cases
where the tty_port_init(&cs->port) call has already been passed.

> @@ -751,14 +752,14 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
>  	gig_dbg(DEBUG_INIT, "setting up iif");
>  	if (gigaset_isdn_regdev(cs, modulename) < 0) {
>  		pr_err("error registering ISDN device\n");
> -		goto error;
> +		goto error_port;
>  	}
>  
>  	make_valid(cs, VALID_ID);
>  	++cs->cs_init;
>  	gig_dbg(DEBUG_INIT, "setting up hw");
>  	if (cs->ops->initcshw(cs) < 0)
> -		goto error;
> +		goto error_port;
>  
>  	++cs->cs_init;
>  
> @@ -773,7 +774,7 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
>  		gig_dbg(DEBUG_INIT, "setting up bcs[%d]", i);
>  		if (gigaset_initbcs(cs->bcs + i, cs, i) < 0) {
>  			pr_err("could not allocate channel %d data\n", i);
> -			goto error;
> +			goto error_port;
>  		}
>  	}
>  
> @@ -786,7 +787,8 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
>  
>  	gig_dbg(DEBUG_INIT, "cs initialized");
>  	return cs;
> -
> +error_port:
> +	tty_port_destroy(&cs->port);
>  error:
>  	gig_dbg(DEBUG_INIT, "failed");
>  	gigaset_freecs(cs);

You have already added a tty_port_destroy() call to gigaset_freecs(cs)
above. Adding another one here will lead to the port being destroyed
twice in this code path.

Thanks,
Tilman

-- 
Tilman Schmidt                    E-Mail: tilman@imap.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 261 bytes --]