staging: comedi: pcmuio: remove an unneeded check

PATCH In review Dan Carpenter 2 participants 3 messages Aug 20, 2013

Subsystems (2)

Staging Kernel janitors

lore

Share & tools

lore

Dan Carpenter

author

2013-08-20 09:00 UTC #0 lore raw

PATCH 1/1 staging: comedi: pcmuio: remove an unneeded check

"devpriv" is non-NULL at this point. We dereference it earlier in the function and the inconsistent checking upsets static checkers. We don't need to check "devpriv->sprivs" because kfree() accepts NULL pointers.

Signed-off-by Dan Carpenter <[email protected]>

drivers/staging/comedi/drivers/pcmuio.c +1 -2

index f942455..a779062 100644
@@ -676,8 +676,7 @@ static void pcmuio_detach(struct comedi_device *dev)
 		if (devpriv->asics[i].irq)
 			free_irq(devpriv->asics[i].irq, dev);
 	}
-	if (devpriv && devpriv->sprivs)
-		kfree(devpriv->sprivs);
+	kfree(devpriv->sprivs);
 	comedi_legacy_detach(dev);
 }

Ian Abbott

2013-08-20 10:02 UTC #1 lore raw

Quoted 12 lines · Dan Carpenter

"devpriv" is non-NULL at this point. We dereference it earlier in the function and the inconsistent checking upsets static checkers. We don't need to check "devpriv->sprivs" because kfree() accepts NULL pointers.
Signed-off-by Dan Carpenter <[email protected]>
drivers/staging/comedi/drivers/pcmuio.c +0 -0
index f942455..a779062 100644

drivers/staging/comedi/drivers/pcmuio.c static void pcmuio_detach(struct comedi_device *dev) · line 682

Quoted 9 lines

drivers/staging/comedi/drivers/pcmuio.c +1 -2

@@ -676,8 +676,7 @@ static void pcmuio_detach(struct comedi_device *dev)
  		if (devpriv->asics[i].irq)
  			free_irq(devpriv->asics[i].irq, dev);
  	}
-	if (devpriv && devpriv->sprivs)
-		kfree(devpriv->sprivs);
+	kfree(devpriv->sprivs);
  	comedi_legacy_detach(dev);
  }

Actually, the function is buggy anyway as devpriv might be NULL (pcmuio_detach() is called even if pcmuio_attach() returns an error).

So let's drop this patch and I'll post a patch to fix the bug.

Signature · 2 lines

-=( Ian Abbott @ MEV Ltd.    E-mail: <[email protected]>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

Dan Carpenter

author

2013-08-20 10:06 UTC #2 lore raw

In reply to Dan Carpenter

Quoted 27 lines · Ian Abbott

Quoted 12 lines · Dan Carpenter
"devpriv" is non-NULL at this point. We dereference it earlier in the function and the inconsistent checking upsets static checkers. We don't need to check "devpriv->sprivs" because kfree() accepts NULL pointers.
Signed-off-by Dan Carpenter <[email protected]>
drivers/staging/comedi/drivers/pcmuio.c +0 -0
index f942455..a779062 100644
drivers/staging/comedi/drivers/pcmuio.c static void pcmuio_detach(struct comedi_device *dev) · line 682
Quoted 9 lines
drivers/staging/comedi/drivers/pcmuio.c +1 -2
@@ -676,8 +676,7 @@ static void pcmuio_detach(struct comedi_device *dev)
 		if (devpriv->asics[i].irq)
 			free_irq(devpriv->asics[i].irq, dev);
 	}
-	if (devpriv && devpriv->sprivs)
-		kfree(devpriv->sprivs);
+	kfree(devpriv->sprivs);
 	comedi_legacy_detach(dev);
 }
 
Actually, the function is buggy anyway as devpriv might be NULL (pcmuio_detach() is called even if pcmuio_attach() returns an error).

Quoted 1 line

So let's drop this patch and I'll post a patch to fix the bug.

Ah. Sorry about that. Thanks for catching this.

regards,
dan carpenter