Re: [PATCH] korina: Read buffer overflow

[Phil Sutter <n0-1@freewrt.org>]

Hi,

On Sat, Aug 08, 2009 at 03:14:57PM +0200, roel kluin wrote:
> A few comments:
> 
> > | diff --git a/drivers/net/korina.c b/drivers/net/korina.c
> > | index a2701f5..d1c5276 100644
> > | --- a/drivers/net/korina.c
> > | +++ b/drivers/net/korina.c
> 
> > | @@ -772,6 +772,13 @@ static void korina_alloc_ring(struct net_device *dev)
> > |               lp->rd_ring[i].ca = CPHYSADDR(skb->data);
> > |               lp->rd_ring[i].link = CPHYSADDR(&lp->rd_ring[i+1]);
> > |       }
> > | +     if (!i) {
> > | +             printk(KERN_ERR DRV_NAME "%s: could not allocate a single"
> > | +                             " receive descriptor\n", dev->name)
> 
>             printk(KERN_ERR "%s: could not allocate a single receive
> descriptor\n",
>                             dev->name);
> 
> Don't interrupt strings, they are an exception in the 80 character
> limit (since they are easier to grep that way).

Ah, good to know.

> Also I think the DRV_NAME should be omitted, Doesn't
> `DRV_NAME "%s:...\n", dev->name)' result in "korinakorina:..."? It
> occurs at more places in the file. Also a semicolon was missing.

Sounds like a valid point, I will check this. This patch was just a
quick hack to illustrate what I had in mind, without even
compile-testing it.

> Otherwise looks fine to me.

Great you like it, and many thanks for reviewing it. I'm still not
completely sure the NIC will still work with less than 64 receive
descriptors, but since the documentation wasn't really enlightening I'll
add some run-time test to my TODO. Maybe one of the other readers has
some advice here, I'll prepare a complete patch in the meantime.

Greetings, Phil