Re: [PATCH v2 25/26] staging: wilc1000: refactor wilc_netdev_init() to handle memory free in error path

[Ajay Singh <ajay.kathat@microchip.com>]

Hi Claudiu,

On Tue, 11 Sep 2018 12:21:13 +0300
Claudiu Beznea <Claudiu.Beznea@microchip.com> wrote:

> On 04.09.2018 09:39, Ajay Singh wrote:
> > Refactor the wilc_netdev_init() to cleanup the memory for error
> > scenario and remove unnecessary 'dev' pointer check.
> > 
> > Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> > ---
> >  drivers/staging/wilc1000/linux_wlan.c             | 36
> > ++++++++++++++++-------
> > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c |  6 +++- 2 files
> > changed, 30 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/staging/wilc1000/linux_wlan.c
> > b/drivers/staging/wilc1000/linux_wlan.c index d7d43fd..91a45a7
> > 100644 --- a/drivers/staging/wilc1000/linux_wlan.c
> > +++ b/drivers/staging/wilc1000/linux_wlan.c
> > @@ -1073,10 +1073,8 @@ int wilc_netdev_init(struct wilc **wilc,
> > struct device *dev, int io_type, INIT_LIST_HEAD(&wl->rxq_head.list);
> >  
> >  	wl->hif_workqueue =
> > create_singlethread_workqueue("WILC_wq");
> > -	if (!wl->hif_workqueue) {
> > -		kfree(wl);
> > -		return -ENOMEM;
> > -	}
> > +	if (!wl->hif_workqueue)
> > +		goto free_wl;
> >  
> >  	register_inetaddr_notifier(&g_dev_notifier);
> >  
> > @@ -1085,7 +1083,7 @@ int wilc_netdev_init(struct wilc **wilc,
> > struct device *dev, int io_type, 
> >  		ndev = alloc_etherdev(sizeof(struct wilc_vif));
> >  		if (!ndev)
> > -			return -ENOMEM;
> > +			goto free_ndev;
> >  
> >  		vif = netdev_priv(ndev);
> >  		memset(vif, 0, sizeof(struct wilc_vif));
> > @@ -1106,15 +1104,13 @@ int wilc_netdev_init(struct wilc **wilc,
> > struct device *dev, int io_type, ndev->netdev_ops =
> > &wilc_netdev_ops; 
> >  		wdev = wilc_create_wiphy(ndev, dev);
> > -
> > -		if (dev)
> > -			SET_NETDEV_DEV(ndev, dev);
> > -
> >  		if (!wdev) {
> >  			netdev_err(ndev, "Can't register WILC
> > Wiphy\n");
> > -			return -1;
> > +			goto free_ndev;
> >  		}
> >  
> > +		SET_NETDEV_DEV(ndev, dev);
> > +
> >  		vif->ndev->ieee80211_ptr = wdev;
> >  		vif->ndev->ml_priv = vif;
> >  		wdev->netdev = vif->ndev;
> > @@ -1125,11 +1121,29 @@ int wilc_netdev_init(struct wilc **wilc,
> > struct device *dev, int io_type, 
> >  		ret = register_netdev(ndev);
> >  		if (ret)
> > -			return ret;
> > +			goto free_ndev;  
> 
> In case this happens you will loose the return code of
> register_netdev() and you will return instead -ENOMEM. Maybe, the
> best approach will be to initialize ret = -ENOMEM while declaring it

Thanks for your suggestion to handle the return code.
I will work on it and submit the changes in different series. 

Regards,
Ajay