[PATCH 2/9] staging: rtl8192e: Fix smatch error and warning in rtl_core.c

Dan Carpenter error27 at gmail.com
Tue Aug 30 17:26:13 UTC 2011


On Tue, Aug 30, 2011 at 11:53:02AM -0500, Larry Finger wrote:
> Smatch shows the following errors:
> 
>   CHECK   drivers/staging/rtl8192e/rtl_core.c
> drivers/staging/rtl8192e/rtl_core.c +600 rtl8192_qos_activate(7) warn: variable dereferenced before check 'priv'
> drivers/staging/rtl8192e/rtl_core.c +1345 rtl8192_init(40) warn: 'dev->irq' was not released on error
> drivers/staging/rtl8192e/rtl_core.c +2120 rtl8192_alloc_rx_desc_ring(43) error: potential null derefence 'entry'.
> drivers/staging/rtl8192e/rtl_core.c +3010 rtl8192_pci_probe(153) warn: 'pmem_start' was not released on error
> 
> Signed-off-by: Larry Finger <Larry.Finger at lwfinger.net>
> ---
>  drivers/staging/rtl8192e/rtl_core.c |   16 +++++++---------
>  1 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/staging/rtl8192e/rtl_core.c b/drivers/staging/rtl8192e/rtl_core.c
> index ce0b2ff..597907f 100644
> --- a/drivers/staging/rtl8192e/rtl_core.c
> +++ b/drivers/staging/rtl8192e/rtl_core.c
> @@ -448,7 +448,7 @@ bool MgntActSet_RF_State(struct net_device *dev,
>  		spin_unlock_irqrestore(&priv->rf_ps_lock, flag);
>  	}
>  
> -	RT_TRACE((COMP_PS && COMP_RF), "<===MgntActSet_RF_State()\n");
> +	RT_TRACE((COMP_PS & COMP_RF), "<===MgntActSet_RF_State()\n");

This stray change was added by mistake.  (Neither & nor && make
sense btw).

>  	return bActionAllowed;
>  }
>  
> @@ -594,11 +594,12 @@ static void rtl8192_qos_activate(void *data)
>  {
>  	struct r8192_priv *priv = container_of_work_rsl(data, struct r8192_priv,
>  				  qos_activate);
> -	struct net_device *dev = priv->rtllib->dev;
> +	struct net_device *dev;
>  	int i;
>  
>  	if (priv == NULL)
>  		return;
> +	dev = priv->rtllib->dev;

It would be better to remove the NULL check.  container_of_work_rsl()
is doing pointer math and it basically can't be zero.  If
->qos_activate were the first element in the r8192_priv struct then
maybe, but with the code how it is, the NULL check can be removed.

>  
>  	mutex_lock(&priv->mutex);
>  	if (priv->rtllib->state != RTLLIB_LINKED)
> @@ -1342,6 +1343,7 @@ static short rtl8192_init(struct net_device *dev)
>  
>  	if (rtl8192_pci_initdescring(dev) != 0) {
>  		printk(KERN_ERR "Endopoints initialization failed");
> +		free_irq(dev->irq, dev);
>  		return -1;
>  	}
>  
> @@ -2117,7 +2119,8 @@ static short rtl8192_alloc_rx_desc_ring(struct net_device *dev)
>  			entry->OWN = 1;
>  		}
>  
> -		entry->EOR = 1;
> +		if(entry)
> +			entry->EOR = 1;
>  	}
>  	return 0;
>  }
> @@ -2988,12 +2991,7 @@ static int __devinit rtl8192_pci_probe(struct pci_dev *pdev,
>  	return 0;
>  
>  fail1:
> -	if (dev->mem_start != (unsigned long)NULL) {
> -		iounmap((void *)dev->mem_start);

Don't we need an iounmap()?

It would be best to just write the error handling in the norma way.

err_unmap:
	iounmap((void *)dev->mem_start);
err_release:
	release_mem_region(pci_resource_start(pdev, 1), pmem_len);
err_irq:
	free_irq(dev->irq, dev);

etc...

regards,
dan carpenter



More information about the devel mailing list