LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* media/radio/radio-si470x.c: check-after-use
@ 2008-01-28 22:13 Adrian Bunk
  2008-01-28 22:43 ` [PATCH] radio-si470x.c: check-after-use Tobias Lorenz
  0 siblings, 1 reply; 4+ messages in thread
From: Adrian Bunk @ 2008-01-28 22:13 UTC (permalink / raw)
  To: Tobias Lorenz, mchehab; +Cc: v4l-dvb-maintainer, linux-kernel

The Coverity checker spotted the following check-after-use in 
drivers/media/radio/radio-si470x.c:

<--  snip  -->

...
static void si470x_usb_driver_disconnect(struct usb_interface *intf)
{
        struct si470x_device *radio = usb_get_intfdata(intf);

        del_timer_sync(&radio->timer);    <------------------
        flush_scheduled_work();

        usb_set_intfdata(intf, NULL);
        if (radio) {                      <------------------
                video_unregister_device(radio->videodev);
                kfree(radio->buffer);
                kfree(radio);
        }
}
...

<--  snip  -->

Either "radio" can be NULL and this case has to be properly handled or 
the NULL check is not required.

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


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

* [PATCH] radio-si470x.c: check-after-use
  2008-01-28 22:13 media/radio/radio-si470x.c: check-after-use Adrian Bunk
@ 2008-01-28 22:43 ` Tobias Lorenz
  2008-01-29 19:37   ` Oliver Neukum
  0 siblings, 1 reply; 4+ messages in thread
From: Tobias Lorenz @ 2008-01-28 22:43 UTC (permalink / raw)
  To: Mauro Carvalho Chehab; +Cc: Adrian Bunk, video4linux-list, linux-kernel

Hi Mauro,
Hi Adrian,

Adrian used the coverity checker against radio-si470x and found this:

> The Coverity checker spotted the following check-after-use in 
> drivers/media/radio/radio-si470x.c:
> 
> <--  snip  -->
> static void si470x_usb_driver_disconnect(struct usb_interface *intf)
> {
>         struct si470x_device *radio = usb_get_intfdata(intf);
> 
>         del_timer_sync(&radio->timer);    <------------------
>         flush_scheduled_work();
> 
>         usb_set_intfdata(intf, NULL);
>         if (radio) {                      <------------------
>                 video_unregister_device(radio->videodev);
>                 kfree(radio->buffer);
>                 kfree(radio);
>         }
> }
> <--  snip  -->
> 
> Either "radio" can be NULL and this case has to be properly handled or 
> the NULL check is not required.

These two lines should indeed better be inside the if statement. The patch for this is below.

Thanks,
  Toby

Signed-off-by: Tobias Lorenz <tobias.lorenz@gmx.net>
--- linux-2.6.23/drivers/media/radio/radio-si470x.c     2008-01-23 00:01:07.000000000 +0100
+++ linux-2.6.23.new/drivers/media/radio/radio-si470x.c 2008-01-27 15:31:42.000000000 +0100
@@ -1440,11 +1440,10 @@ static void si470x_usb_driver_disconnect
 {
        struct si470x_device *radio = usb_get_intfdata(intf);

-       del_timer_sync(&radio->timer);
-       flush_scheduled_work();
-
        usb_set_intfdata(intf, NULL);
        if (radio) {
+               del_timer_sync(&radio->timer);
+               flush_scheduled_work();
                video_unregister_device(radio->videodev);
                kfree(radio->buffer);
                kfree(radio);

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

* Re: [PATCH] radio-si470x.c: check-after-use
  2008-01-28 22:43 ` [PATCH] radio-si470x.c: check-after-use Tobias Lorenz
@ 2008-01-29 19:37   ` Oliver Neukum
  2008-01-29 20:37     ` Tobias Lorenz
  0 siblings, 1 reply; 4+ messages in thread
From: Oliver Neukum @ 2008-01-29 19:37 UTC (permalink / raw)
  To: Tobias Lorenz
  Cc: Mauro Carvalho Chehab, Adrian Bunk, video4linux-list, linux-kernel

Am Montag 28 Januar 2008 schrieb Tobias Lorenz:
> > Either "radio" can be NULL and this case has to be properly handled or 
> > the NULL check is not required.
> 
> These two lines should indeed better be inside the if statement. The patch for this is below.

No, in disconnect intfdata must be valid. Any check for NULL is wrong
there.

	Regards
		Oliver


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

* Re: [PATCH] radio-si470x.c: check-after-use
  2008-01-29 19:37   ` Oliver Neukum
@ 2008-01-29 20:37     ` Tobias Lorenz
  0 siblings, 0 replies; 4+ messages in thread
From: Tobias Lorenz @ 2008-01-29 20:37 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Mauro Carvalho Chehab, Adrian Bunk, video4linux-list, linux-kernel

Hi,

> > > Either "radio" can be NULL and this case has to be properly handled or 
> > > the NULL check is not required.
> > 
> > These two lines should indeed better be inside the if statement. The patch for this is below.
> 
> No, in disconnect intfdata must be valid. Any check for NULL is wrong
> there.

Why hadn't anybody told me that earlier? :-)
I removed the complete check for radio==NULL.
See the patch below.

Thanks,
  Toby


Signed-off-by: Tobias Lorenz <tobias.lorenz@gmx.net>
--- linux-2.6.23/drivers/media/radio/radio-si470x.c     2008-01-23 00:01:07.000000000 +0100
+++ linux-2.6.23.new/drivers/media/radio/radio-si470x.c 2008-01-27 15:31:42.000000000 +0100
@@ -1441,13 +1441,11 @@ static void si470x_usb_driver_disconnect
        struct si470x_device *radio = usb_get_intfdata(intf);

        usb_set_intfdata(intf, NULL);
-       if (radio) {
-               del_timer_sync(&radio->timer);
-               flush_scheduled_work();
-               video_unregister_device(radio->videodev);
-               kfree(radio->buffer);
-               kfree(radio);
-       }
+       del_timer_sync(&radio->timer);
+       flush_scheduled_work();
+       video_unregister_device(radio->videodev);
+       kfree(radio->buffer);
+       kfree(radio);
 }

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

end of thread, other threads:[~2008-01-29 20:37 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-01-28 22:13 media/radio/radio-si470x.c: check-after-use Adrian Bunk
2008-01-28 22:43 ` [PATCH] radio-si470x.c: check-after-use Tobias Lorenz
2008-01-29 19:37   ` Oliver Neukum
2008-01-29 20:37     ` Tobias Lorenz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).