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@xxxxxxxxxxxx> > --- > 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 devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel