lkml.org 
[lkml]   [2022]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2 05/13] can: slcan: simplify the device de-allocation
From
Date
This patch (at least) needs some rework.

The patch cf124db566e6b036 ("net: Fix inconsistent teardown and release
of private netdev state.") from DaveM added some priv_destructor

dev->priv_destructor = sl_free_netdev;

which is not taken into account in this patch.

As written before I would like to discuss this change out of your patch
series "can: slcan: extend supported features" as it is no slcan feature
extension AND has to be synchronized with the drivers/net/slip/slip.c
implementation.

When it has not real benefit and introduces more code and may create
side effects, this beautification should probably be omitted at all.

Thanks,
Oliver

On 08.06.22 18:51, Dario Binacchi wrote:
> Since slcan_devs array contains the addresses of the created devices, I
> think it is more natural to use its address to remove it from the list.
> It is not necessary to store the index of the array that points to the
> device in the driver's private data.
>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> ---
>
> (no changes since v1)
>
> drivers/net/can/slcan.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
> index 929cb55e08af..cf05c30b8da5 100644
> --- a/drivers/net/can/slcan.c
> +++ b/drivers/net/can/slcan.c
> @@ -432,11 +432,17 @@ static int slc_open(struct net_device *dev)
>
> static void slc_dealloc(struct slcan *sl)
> {
> - int i = sl->dev->base_addr;
> + unsigned int i;
>
> - free_candev(sl->dev);
> - if (slcan_devs)
> - slcan_devs[i] = NULL;
> + for (i = 0; i < maxdev; i++) {
> + if (sl->dev == slcan_devs[i]) {
> + free_candev(sl->dev);
> + slcan_devs[i] = NULL;
> + return;
> + }
> + }
> +
> + pr_err("slcan: can't free %s resources\n", sl->dev->name);
> }
>
> static int slcan_change_mtu(struct net_device *dev, int new_mtu)
> @@ -533,7 +539,6 @@ static struct slcan *slc_alloc(void)
>
> snprintf(dev->name, sizeof(dev->name), "slcan%d", i);
> dev->netdev_ops = &slc_netdev_ops;
> - dev->base_addr = i;
> sl = netdev_priv(dev);
>
> /* Initialize channel control data */
>

\
 
 \ /
  Last update: 2022-06-08 22:39    [W:0.121 / U:0.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site