On Thu, 2008-09-11 at 02:34 -0400, Peter Memishian wrote:
> > I'm going to integrate the following into clearview-ipobs tonight:
>  > 
>  > http://zhadum.east.sun.com/ws/seb/seb-ipobs/webrev.plumbing/
> 
> Looks good.  One thing that might pretty up the code a bit would be to
> pass a boolean `isv6' variable to the ipnetif_*_ev() functions rather than
> the family.  Similar changes could be made to ipnet_populate_if() and
> related functions.

Done.

> On line 929, it doesn't seem we check for an unspecified address, despite
> the comment.

That needs to be removed.  The unspecified check is for the subnet
broadcast address down in the loop.

> In ipnet_create_if(), do we still need `where' if we expect avl_find() to
> return NULL?

Yes, because that is the position in the tree that gets passed in to
avl_insert() on the next line.

> As an aside, it might clean up the code to have some #defines for
> ifa_ip4addr and ifa_ip6addr like we do for iapu_addr4 and iapu_addr6
> (and it'd be nice if they followed the same naming convention for
> their fields).

Fixed.

Thanks,
-Seb



Reply via email to