netdev
[Top] [All Lists]

Re: gred_dump (2.4.17): bad semantic and memory leak

To: jamal <hadi@xxxxxxxxxx>
Subject: Re: gred_dump (2.4.17): bad semantic and memory leak
From: Martin Devera <devik@xxxxxx>
Date: Sat, 19 Jan 2002 20:14:41 +0100 (CET)
Cc: netdev@xxxxxxxxxxx, Alexander Atanasov <alex@xxxxxx>
In-reply-to: <Pine.GSO.4.30.0201181830290.18875-100000@shell.cyberus.ca>
Sender: owner-netdev@xxxxxxxxxxx
> Ok, is it sch_gred day or what? ;-> Martin you had to get me out hiding,
> didnt you?

Hehe, I have to keep you in touch with life .. I need you
to look over my paper ;-)

> Please look for more bugs before i submit ;->
> 
> BTW, i dont think that is a semantical problem, its just not clean so i
> cleaned that too and fixed an email address i havent repsonded to for
> about a year now ;->

well, look below ..
 
>       if (!table->initd) {
>               DPRINTK("NO GRED Queues setup!\n");
> -             return -1;
> +             goto rtattr_failure;
>       }
> 

I think that you should not fail so hardly here. I'm not sure what
is table->initd for but it is possible that user configures it in way
where table->initd is NULL (actualy one user did it and complained
about HTB error - this way I found bug above).
Dump in sch_api.c calls xxx_dump for all qdiscs on the interface until
all are exhausted or -1 is returned.
By returning -1 in !table->initd case you prevent all other qdisc
from being displayed. IMHO the -1 value is used for hard error
(eg. unrecoverable one in RTNETLINK comm) not to report error in
qdisc's setup.
Do you think that it makes sense ?

devik


<Prev in Thread] Current Thread [Next in Thread>