On Fri, Jul 22, 2011 at 02:51:32PM +0200, Martin Pelikan wrote:
> Hi,
> 
> this panic cannot possibly happen. The question is, whether to move it up
> to the macro that is used in the code, or zap it entirely.
> The whole logic in these two seems pretty ugly, but inet6 crashed when I 
> tried to make it look prettier and more uniform.  And again, until 5.0 is
> out, this might just sit here, or wait on someone's todo list.
> 
> --
> Martin Pelikan
> 
> Index: net/if.h
> ===================================================================
> RCS file: /cvs/src/sys/net/if.h,v
> retrieving revision 1.128
> diff -u -p -r1.128 if.h
> --- net/if.h  8 Jul 2011 18:48:51 -0000       1.128
> +++ net/if.h  22 Jul 2011 12:14:01 -0000
> @@ -696,6 +696,8 @@ __END_DECLS
>  #ifdef _KERNEL
>  #define      IFAFREE(ifa) \
>  do { \
> +     if (ifa == NULL) \
> +             panic("IFAFREE"); \
>       if ((ifa)->ifa_refcnt <= 0) \
>               ifafree(ifa); \
>       else \
> Index: net/route.c
> ===================================================================
> RCS file: /cvs/src/sys/net/route.c,v
> retrieving revision 1.131
> diff -u -p -r1.131 route.c
> --- net/route.c       4 Jul 2011 04:29:17 -0000       1.131
> +++ net/route.c       22 Jul 2011 12:14:01 -0000
> @@ -411,8 +411,6 @@ rtfree(struct rtentry *rt)
>  void
>  ifafree(struct ifaddr *ifa)
>  {
> -     if (ifa == NULL)
> -             panic("ifafree");
>       if (ifa->ifa_refcnt == 0)
>               free(ifa, M_IFADDR);
>       else
> 

What is the point of the #define? If anything I'd go the other way
and move the refcount check into ifafree(). Saving a function call
at the expense of bloating the kernel has, in other cases, been
shown to be a losing proposition. I can only see fewer than 30
IFAFREE() occurances in /usr/src/*.c.

.... Ken

Reply via email to