[quagga-dev 253] Re: PATCH: suspected "reference leak" in zebra/zerba_rib.c

Gilad Arnold gilad.arnold at terayon.com
Wed Sep 24 11:10:32 BST 2003

Hi Paul,

Paul Jakma wrote:

> Index: zebra/zebra_rib.c
> ===================================================================
> RCS file: /var/cvsroot/quagga/zebra/zebra_rib.c,v
> retrieving revision 1.11
> diff -c -u -r1.11 zebra_rib.c
> --- zebra/zebra_rib.c	15 Jul 2003 12:52:22 -0000	1.11
> +++ zebra/zebra_rib.c	24 Sep 2003 08:43:30 -0000
> @@ -1575,11 +1575,14 @@
>      rn->info = si->next;
>    if (si->next)
>      si->next->prev = si->prev;
> +  route_unlock_node (rn);
>    /* Free static route configuration. */
>    if (ifname)
>      XFREE (0, si->gate.ifname);
> +
> +  route_unlock_node (rn);
>    return 1;
>  }
> interesting.. why is it locked twice?

Once due to route_node_lookup(), which locks the matching node before 
returning it; twice because you remove an element (static route, in this 
case) from the node's info structure. The unlock calls I've added refer 
to the latter, then the former, in that order (you could of course bunch 
them up before the return of the function, I figured it's more readable 
this way though). Basically I've just reversed the effect of 

Does it make sense to you, or is there anything else you see?

> on a more abstract, long-term, wishful-thinking vain: I'd love to 
> eradicate a lot of the implicit locking which is done in table.c. Let 
> the caller decide, something worth doing?

I don't really agree: this locking may seem too strict from time to 
time, however it does assure you that once you've grabbed a reference to 
some route node, by some call to the table lib, it can't be invalidated 
until you explicitly let go of it. Why would you want to do that?

(And, how's everything? ;->  I seem to have paused my work on recursive 
multihop for the time being, I'm extremely overloaded these days, 
apologies for that...)


More information about the Quagga-dev mailing list