Re: [CHECKER] 6 memory leaks

Jean Tourrilhes
2003-04-21T20:15:37Z

William Lee Irwin III wrote :
>
> On Sat, Apr 19, 2003 at 12:44:45PM +0300, Muli Ben-Yehuda wrote:
> > Index: net/irda/irttp.c
> > ===================================================================
> > RCS file: /home/cvs/linux-2.5/net/irda/irttp.c,v
> > retrieving revision 1.12
> > diff -u -r1.12 irttp.c
> > --- net/irda/irttp.c 25 Feb 2003 05:02:46 -0000 1.12
> > +++ net/irda/irttp.c 19 Apr 2003 08:50:00 -0000
> > @@ -263,7 +263,7 @@
> >
> > IRDA_DEBUG(2, "%s(), rx_sdu_size=%d\n", __FUNCTION__,
> > self->rx_sdu_size);
> > - ASSERT(n <= self->rx_sdu_size, return NULL;);
> > + ASSERT(n <= self->rx_sdu_size, {dev_kfree_skb(skb); return NULL;});
> >
> > /* Set the new length */
> > skb_trim(skb, n);

Thanks for the heads up. I'm preparing a massive skb leak
patch for 2.5.X, I'll slip that into it. I'll probably code that
differently so that it looks "cleaner".
By the way, this is not terribly important, as if ASSERT do
trigger we usually have bigger problems than memory leaks (like you
may want to reboot rather sooner than later).

> I'm in terror. ASSERT()? return NULL in a macro argument?
> Any chance of cleaning that up a bit while you're at it?
>
> -- wli

Rather than fixing imaginary non-existing bugs, I prefer to
spend my time fixing real bugs that byte real users. This construct is
perfectly sound and valid, and it needs to be done in this way, the
only issue is that someone should rename "ASSERT" into "IRDA_ASSERT".

Have fun...

Jean