netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 3/6] drivers/net: remove null pointer dereference
@ 2008-05-12 13:37 Julia Lawall
  2008-05-12 16:17 ` Francois Romieu
  0 siblings, 1 reply; 7+ messages in thread
From: Julia Lawall @ 2008-05-12 13:37 UTC (permalink / raw)
  To: jgarzik, netdev, linux-kernel, kernel-janitors

From: Julia Lawall <julia@diku.dk>

If dev is NULL, it is not possible to access its name field.  So I have
simply modified the error message to drop the printing of the name field.


This problem was found using the following semantic match
(http://www.emn.fr/x-info/coccinelle/)

// <smpl>
@@
expression E, E1;
identifier f;
statement S1,S2,S3;
@@

* if (E == NULL)
{
  ... when != if (E == NULL) S1 else S2
      when != E = E1
* E->f
  ... when any
  return ...;
}
else S3
// </smpl>

Signed-off-by: Julia Lawall <julia@diku.dk>

---

diff -u -p a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
--- a/drivers/net/au1000_eth.c	2008-04-27 11:41:11.000000000 +0200
+++ b/drivers/net/au1000_eth.c	2008-05-12 09:32:54.000000000 +0200
@@ -1242,7 +1242,7 @@ static irqreturn_t au1000_interrupt(int 
 	struct net_device *dev = (struct net_device *) dev_id;
 
 	if (dev == NULL) {
-		printk(KERN_ERR "%s: isr: null dev ptr\n", dev->name);
+		printk(KERN_ERR "isr: null dev ptr\n");
 		return IRQ_RETVAL(1);
 	}
 

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

* Re: [PATCH 3/6] drivers/net: remove null pointer dereference
  2008-05-12 13:37 [PATCH 3/6] drivers/net: remove null pointer dereference Julia Lawall
@ 2008-05-12 16:17 ` Francois Romieu
  2008-05-12 16:53   ` Julia Lawall
  0 siblings, 1 reply; 7+ messages in thread
From: Francois Romieu @ 2008-05-12 16:17 UTC (permalink / raw)
  To: Julia Lawall; +Cc: jgarzik, netdev, linux-kernel, kernel-janitors

Julia Lawall <julia@diku.dk> :
[...]
> diff -u -p a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
> --- a/drivers/net/au1000_eth.c	2008-04-27 11:41:11.000000000 +0200
> +++ b/drivers/net/au1000_eth.c	2008-05-12 09:32:54.000000000 +0200
> @@ -1242,7 +1242,7 @@ static irqreturn_t au1000_interrupt(int 
>  	struct net_device *dev = (struct net_device *) dev_id;
>  
>  	if (dev == NULL) {
> -		printk(KERN_ERR "%s: isr: null dev ptr\n", dev->name);
> +		printk(KERN_ERR "isr: null dev ptr\n");
>  		return IRQ_RETVAL(1);
>  	}

The lifespan of 'dev' covers the request_irq..free_irq interval in this
driver. The whole 'dev == NULL' block can be removed.

-- 
Ueimor

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

* [PATCH] au1000_eth: remove useless check
  2008-05-12 16:53   ` Julia Lawall
@ 2008-05-12 16:44     ` Francois Romieu
  2008-05-22 10:22       ` Jeff Garzik
  2008-05-12 17:06     ` [PATCH 3/6] drivers/net: remove null pointer dereference Jeff Garzik
  1 sibling, 1 reply; 7+ messages in thread
From: Francois Romieu @ 2008-05-12 16:44 UTC (permalink / raw)
  To: Julia Lawall; +Cc: jgarzik, netdev, linux-kernel, kernel-janitors

The lifespan of the device covers the request_irq .. free_irq interval.

The cast of a void * pointer is not needed either.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>

diff --git a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
index 3634b5f..7023d77 100644
--- a/drivers/net/au1000_eth.c
+++ b/drivers/net/au1000_eth.c
@@ -1239,12 +1239,7 @@ static int au1000_rx(struct net_device *dev)
  */
 static irqreturn_t au1000_interrupt(int irq, void *dev_id)
 {
-	struct net_device *dev = (struct net_device *) dev_id;
-
-	if (dev == NULL) {
-		printk(KERN_ERR "%s: isr: null dev ptr\n", dev->name);
-		return IRQ_RETVAL(1);
-	}
+	struct net_device *dev = dev_id;
 
 	/* Handle RX interrupts first to minimize chance of overrun */
 

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

* Re: [PATCH 3/6] drivers/net: remove null pointer dereference
  2008-05-12 16:17 ` Francois Romieu
@ 2008-05-12 16:53   ` Julia Lawall
  2008-05-12 16:44     ` [PATCH] au1000_eth: remove useless check Francois Romieu
  2008-05-12 17:06     ` [PATCH 3/6] drivers/net: remove null pointer dereference Jeff Garzik
  0 siblings, 2 replies; 7+ messages in thread
From: Julia Lawall @ 2008-05-12 16:53 UTC (permalink / raw)
  To: Francois Romieu; +Cc: jgarzik, netdev, linux-kernel, kernel-janitors

On Mon, 12 May 2008, Francois Romieu wrote:

> Julia Lawall <julia@diku.dk> :
> [...]
> > diff -u -p a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
> > --- a/drivers/net/au1000_eth.c	2008-04-27 11:41:11.000000000 +0200
> > +++ b/drivers/net/au1000_eth.c	2008-05-12 09:32:54.000000000 +0200
> > @@ -1242,7 +1242,7 @@ static irqreturn_t au1000_interrupt(int 
> >  	struct net_device *dev = (struct net_device *) dev_id;
> >  
> >  	if (dev == NULL) {
> > -		printk(KERN_ERR "%s: isr: null dev ptr\n", dev->name);
> > +		printk(KERN_ERR "isr: null dev ptr\n");
> >  		return IRQ_RETVAL(1);
> >  	}
> 
> The lifespan of 'dev' covers the request_irq..free_irq interval in this
> driver. The whole 'dev == NULL' block can be removed.

Will you do that?

julia

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

* Re: [PATCH 3/6] drivers/net: remove null pointer dereference
  2008-05-12 16:53   ` Julia Lawall
  2008-05-12 16:44     ` [PATCH] au1000_eth: remove useless check Francois Romieu
@ 2008-05-12 17:06     ` Jeff Garzik
  2008-05-12 18:48       ` Julia Lawall
  1 sibling, 1 reply; 7+ messages in thread
From: Jeff Garzik @ 2008-05-12 17:06 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Francois Romieu, netdev, linux-kernel, kernel-janitors

Julia Lawall wrote:
> On Mon, 12 May 2008, Francois Romieu wrote:
> 
>> Julia Lawall <julia@diku.dk> :
>> [...]
>>> diff -u -p a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
>>> --- a/drivers/net/au1000_eth.c	2008-04-27 11:41:11.000000000 +0200
>>> +++ b/drivers/net/au1000_eth.c	2008-05-12 09:32:54.000000000 +0200
>>> @@ -1242,7 +1242,7 @@ static irqreturn_t au1000_interrupt(int 
>>>  	struct net_device *dev = (struct net_device *) dev_id;
>>>  
>>>  	if (dev == NULL) {
>>> -		printk(KERN_ERR "%s: isr: null dev ptr\n", dev->name);
>>> +		printk(KERN_ERR "isr: null dev ptr\n");
>>>  		return IRQ_RETVAL(1);
>>>  	}
>> The lifespan of 'dev' covers the request_irq..free_irq interval in this
>> driver. The whole 'dev == NULL' block can be removed.
> 
> Will you do that?

It's normal within the Linux community to give feedback on patches, and 
sometimes the authors need to revise their patches if helpful feedback 
arises.

	Jeff




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

* Re: [PATCH 3/6] drivers/net: remove null pointer dereference
  2008-05-12 17:06     ` [PATCH 3/6] drivers/net: remove null pointer dereference Jeff Garzik
@ 2008-05-12 18:48       ` Julia Lawall
  0 siblings, 0 replies; 7+ messages in thread
From: Julia Lawall @ 2008-05-12 18:48 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: Francois Romieu, netdev, linux-kernel, kernel-janitors

On Mon, 12 May 2008, Jeff Garzik wrote:

> Julia Lawall wrote:
> > On Mon, 12 May 2008, Francois Romieu wrote:
> > 
> > > Julia Lawall <julia@diku.dk> :
> > > [...]
> > > > diff -u -p a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
> > > > --- a/drivers/net/au1000_eth.c	2008-04-27 11:41:11.000000000 +0200
> > > > +++ b/drivers/net/au1000_eth.c	2008-05-12 09:32:54.000000000 +0200
> >>> @@ -1242,7 +1242,7 @@ static irqreturn_t au1000_interrupt(int 
> > > >   struct net_device *dev = (struct net_device *) dev_id;
> > > >  
> > > > 	if (dev == NULL) {
> > > > -		printk(KERN_ERR "%s: isr: null dev ptr\n", dev->name);
> > > > +		printk(KERN_ERR "isr: null dev ptr\n");
> > > >   	return IRQ_RETVAL(1);
> > > >   }
> > > The lifespan of 'dev' covers the request_irq..free_irq interval in this
> > > driver. The whole 'dev == NULL' block can be removed.
> > 
> > Will you do that?
> 
> It's normal within the Linux community to give feedback on patches, and
> sometimes the authors need to revise their patches if helpful feedback arises.

Sorry.  In principle, I would be happy to do it, but I felt slightly 
uneasy about doing something I didn't completely understand.

julia

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

* Re: [PATCH] au1000_eth: remove useless check
  2008-05-12 16:44     ` [PATCH] au1000_eth: remove useless check Francois Romieu
@ 2008-05-22 10:22       ` Jeff Garzik
  0 siblings, 0 replies; 7+ messages in thread
From: Jeff Garzik @ 2008-05-22 10:22 UTC (permalink / raw)
  To: Francois Romieu; +Cc: Julia Lawall, netdev, linux-kernel, kernel-janitors

Francois Romieu wrote:
> The lifespan of the device covers the request_irq .. free_irq interval.
> 
> The cast of a void * pointer is not needed either.
> 
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> 
> diff --git a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
> index 3634b5f..7023d77 100644
> --- a/drivers/net/au1000_eth.c
> +++ b/drivers/net/au1000_eth.c
> @@ -1239,12 +1239,7 @@ static int au1000_rx(struct net_device *dev)
>   */
>  static irqreturn_t au1000_interrupt(int irq, void *dev_id)
>  {
> -	struct net_device *dev = (struct net_device *) dev_id;
> -
> -	if (dev == NULL) {
> -		printk(KERN_ERR "%s: isr: null dev ptr\n", dev->name);
> -		return IRQ_RETVAL(1);
> -	}
> +	struct net_device *dev = dev_id;
>  

applied



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

end of thread, other threads:[~2008-05-22 10:22 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-12 13:37 [PATCH 3/6] drivers/net: remove null pointer dereference Julia Lawall
2008-05-12 16:17 ` Francois Romieu
2008-05-12 16:53   ` Julia Lawall
2008-05-12 16:44     ` [PATCH] au1000_eth: remove useless check Francois Romieu
2008-05-22 10:22       ` Jeff Garzik
2008-05-12 17:06     ` [PATCH 3/6] drivers/net: remove null pointer dereference Jeff Garzik
2008-05-12 18:48       ` Julia Lawall

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).