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 <[email protected]>
> ---
> 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
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
