public inbox for kernel-janitors@vger.kernel.org
 help / color / mirror / Atom feed
* [patch] staging: comedi: pcmuio: remove an unneeded check
@ 2013-08-20  9:00 Dan Carpenter
  2013-08-20 10:02 ` Ian Abbott
  2013-08-20 10:06 ` Dan Carpenter
  0 siblings, 2 replies; 3+ messages in thread
From: Dan Carpenter @ 2013-08-20  9:00 UTC (permalink / raw)
  To: kernel-janitors

"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 <dan.carpenter@oracle.com>

diff --git a/drivers/staging/comedi/drivers/pcmuio.c b/drivers/staging/comedi/drivers/pcmuio.c
index f942455..a779062 100644
--- a/drivers/staging/comedi/drivers/pcmuio.c
+++ b/drivers/staging/comedi/drivers/pcmuio.c
@@ -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);
 }
 

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [patch] staging: comedi: pcmuio: remove an unneeded check
  2013-08-20  9:00 [patch] staging: comedi: pcmuio: remove an unneeded check Dan Carpenter
@ 2013-08-20 10:02 ` Ian Abbott
  2013-08-20 10:06 ` Dan Carpenter
  1 sibling, 0 replies; 3+ messages in thread
From: Ian Abbott @ 2013-08-20 10:02 UTC (permalink / raw)
  To: kernel-janitors

On 2013-08-20 10:00, Dan Carpenter wrote:
> "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 <dan.carpenter@oracle.com>
>
> diff --git a/drivers/staging/comedi/drivers/pcmuio.c b/drivers/staging/comedi/drivers/pcmuio.c
> index f942455..a779062 100644
> --- a/drivers/staging/comedi/drivers/pcmuio.c
> +++ b/drivers/staging/comedi/drivers/pcmuio.c
> @@ -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.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [patch] staging: comedi: pcmuio: remove an unneeded check
  2013-08-20  9:00 [patch] staging: comedi: pcmuio: remove an unneeded check Dan Carpenter
  2013-08-20 10:02 ` Ian Abbott
@ 2013-08-20 10:06 ` Dan Carpenter
  1 sibling, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2013-08-20 10:06 UTC (permalink / raw)
  To: kernel-janitors

On Tue, Aug 20, 2013 at 11:02:45AM +0100, Ian Abbott wrote:
> On 2013-08-20 10:00, Dan Carpenter wrote:
> >"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 <dan.carpenter@oracle.com>
> >
> >diff --git a/drivers/staging/comedi/drivers/pcmuio.c b/drivers/staging/comedi/drivers/pcmuio.c
> >index f942455..a779062 100644
> >--- a/drivers/staging/comedi/drivers/pcmuio.c
> >+++ b/drivers/staging/comedi/drivers/pcmuio.c
> >@@ -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.

Ah.  Sorry about that.  Thanks for catching this.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-08-20 10:06 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-08-20  9:00 [patch] staging: comedi: pcmuio: remove an unneeded check Dan Carpenter
2013-08-20 10:02 ` Ian Abbott
2013-08-20 10:06 ` Dan Carpenter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox