On 21/11/06, David Rientjes <[email protected]> wrote:
On Tue, 21 Nov 2006, Jesper Juhl wrote:
> Ok, I see your point. There may not be an actual bug here, but
> couldn't it still be considered an improvement, given that with my
> patch we'll a) print a warning that we ran into a memory shortage
> problem, and b) we save a call to ipc->decompress() and some switch
> logic in the failing case. ???
>
No, because there is duplication of code. This error condition is already
addressed:
skb_out = dev_alloc_skb(is->mru + PPP_HDRLEN);
len = ipc->decompress(stat, skb, skb_out, &rsparm);
kfree_skb(skb);
if (len <= 0) {
switch (len) {
case DECOMP_ERROR:
...
break;
case DECOMP_FATALERROR:
...
break;
}
kfree_skb(skb_out);
return NULL;
}
Since neither DECOMP_ERROR or DECOMP_FATALERROR represent a NULL return
from ipc->decompress(), the switch clause is a no-op and skb_out is freed
and NULL is returned.
The only thing your patch addresses is moving this before the
ipc->decompress() call and _duplicating_ both the skb free and the return
code, as well as adding a warning. The warning is unnecessary because OOM
killer will be called soon anyway if this condition is ever reached so the
fact that it was this allocation that could not be satisfied doesn't
matter. Likewise, we need the return code from ipc->decompress() to do
the other error checking involved.
You are right. I concede your point.
--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
