>Number:         33231
>Category:       kern
>Synopsis:       Code cleanup ( route.c )
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Mon Apr 10 18:00:00 +0000 2006
>Originator:     Arnaud Degroote
>Release:        current
>Organization:
>Environment:
NetBSD Hermes.at.home 3.99.17 NetBSD 3.99.17 (HERMES) #2: Mon Apr 10 00:08:34 CEST 2006  zul@Hermes.at.home:/ext/build/objdir/sys/arch/i386/compile/HERMES i386

>Description:
There a lot of ugly things into the current route code.
I just try to correct some basics issues :
   - use of 0 instead of NULL
   - questionnable macro
   - abuse of ternary operator
>How-To-Repeat:
Code reading
>Fix:
The patch fixes some issues with the route.c code.

--- route.c.orig        2006-04-09 17:12:03.000000000 +0200
+++ route.c     2006-04-10 00:08:07.000000000 +0200
@@ -126,8 +126,6 @@
 #include <netns/ns.h>
 #endif

-#define        SA(p) ((struct sockaddr *)(p))
-
 struct route_cb route_cb;
 struct rtstat  rtstat;
 struct radix_node_head *rt_tables[AF_MAX+1];
@@ -179,7 +177,7 @@
        struct radix_node_head *rnh = rt_tables[dst->sa_family];
        struct rtentry *rt;
        struct radix_node *rn;
-       struct rtentry *newrt = 0;
+       struct rtentry *newrt = NULL;
        struct rt_addrinfo info;
        int  s = splsoftnet(), err = 0, msgtype = RTM_MISS;

@@ -187,8 +185,8 @@
            ((rn->rn_flags & RNF_ROOT) == 0)) {
                newrt = rt = (struct rtentry *)rn;
                if (report && (rt->rt_flags & RTF_CLONING)) {
-                       err = rtrequest(RTM_RESOLVE, dst, SA(0),
-                                             SA(0), 0, &newrt);
+                       err = rtrequest(RTM_RESOLVE, dst, NULL, 
+                                             NULL, 0, &newrt);
                        if (err) {
                                newrt = rt;
                                rt->rt_refcnt++;
@@ -228,7 +226,7 @@
 {
        struct ifaddr *ifa;

-       if (rt == 0)
+       if (rt == NULL)
                panic("rtfree");
        rt->rt_refcnt--;
        if (rt->rt_refcnt <= 0 && (rt->rt_flags & RTF_UP) == 0) {
@@ -278,12 +276,12 @@
 {
        struct rtentry *rt;
        int error = 0;
-       u_quad_t *stat = 0;
+       u_quad_t *stat = NULL;
        struct rt_addrinfo info;
        struct ifaddr *ifa;

        /* verify the gateway is directly reachable */
-       if ((ifa = ifa_ifwithnet(gateway)) == 0) {
+       if ((ifa = ifa_ifwithnet(gateway)) == NULL) {
                error = ENETUNREACH;
                goto out;
        }
@@ -310,7 +308,7 @@
         * which use routing redirects generated by smart gateways
         * to dynamically build the routing tables.
         */
-       if ((rt == 0) || (rt_mask(rt) && rt_mask(rt)->sa_len < 2))
+       if ((rt == NULL) || (rt_mask(rt) && rt_mask(rt)->sa_len < 2))
                goto create;
        /*
         * Don't listen to the redirect if it's
@@ -446,10 +444,10 @@
                 * as our clue to the interface.  Otherwise
                 * we can use the local address.
                 */
-               ifa = 0;
+               ifa = NULL;
                if (flags & RTF_HOST)
                        ifa = ifa_ifwithdstaddr(dst);
-               if (ifa == 0)
+               if (ifa == NULL)
                        ifa = ifa_ifwithaddr(gateway);
        } else {
                /*
@@ -459,20 +457,20 @@
                 */
                ifa = ifa_ifwithdstaddr(gateway);
        }
-       if (ifa == 0)
+       if (ifa == NULL)
                ifa = ifa_ifwithnet(gateway);
-       if (ifa == 0) {
+       if (ifa == NULL) {
                struct rtentry *rt = rtalloc1(dst, 0);
-               if (rt == 0)
-                       return (0);
+               if (rt == NULL)
+                       return (NULL);
                rt->rt_refcnt--;
-               if ((ifa = rt->rt_ifa) == 0)
-                       return (0);
+               if ((ifa = rt->rt_ifa) == NULL)
+                       return (NULL);
        }
        if (ifa->ifa_addr->sa_family != dst->sa_family) {
                struct ifaddr *oifa = ifa;
                ifa = ifaof_ifpforaddr(dst, ifa->ifa_ifp);
-               if (ifa == 0)
+               if (ifa == NULL)
                        ifa = oifa;
        }
        return (ifa);
@@ -494,22 +492,16 @@
        return rtrequest1(req, &info, ret_nrt);
 }

-/*
- * These (questionable) definitions of apparent local variables apply
- * to the next function.  XXXXXX!!!
- */
-#define dst    info->rti_info[RTAX_DST]
-#define gateway        info->rti_info[RTAX_GATEWAY]
-#define netmask        info->rti_info[RTAX_NETMASK]
-#define ifaaddr        info->rti_info[RTAX_IFA]
-#define ifpaddr        info->rti_info[RTAX_IFP]
-#define flags  info->rti_flags
-
 int
 rt_getifa(struct rt_addrinfo *info)
 {
        struct ifaddr *ifa;
        int error = 0;
+    const struct sockaddr * dst = info->rti_info[RTAX_DST];
+    const struct sockaddr * gateway = info->rti_info[RTAX_GATEWAY];
+    const struct sockaddr * ifaaddr = info->rti_info[RTAX_IFA];
+    const struct sockaddr * ifpaddr = info->rti_info[RTAX_IFP];
+    int flags = info->rti_flags;

        /*
         * ifp may be specified by sockaddr_dl when protocol address
@@ -524,8 +516,13 @@
        if (info->rti_ifa == NULL) {
                const struct sockaddr *sa;

-               sa = ifaaddr != NULL ? ifaaddr :
-                   (gateway != NULL ? gateway : dst);
+        if ( ifaaddr != NULL ) {
+            sa = ifaaddr;
+        } else if ( gateway != NULL ) {
+            sa = gateway;
+        } else {
+            sa = dst;
+        }
                if (sa != NULL && info->rti_ifp != NULL)
                        info->rti_ifa = ifaof_ifpforaddr(sa, info->rti_ifp);
                else if (dst != NULL && gateway != NULL)
@@ -552,33 +549,37 @@
        struct ifaddr *ifa;
        struct sockaddr *ndst;
        struct sockaddr_storage deldst;
+    const struct sockaddr * dst = info->rti_info[RTAX_DST];
+    const struct sockaddr * gateway = info->rti_info[RTAX_GATEWAY];
+    const struct sockaddr * netmask = info->rti_info[RTAX_NETMASK];
+    int flags = info->rti_flags;
 #define senderr(x) { error = x ; goto bad; }

-       if ((rnh = rt_tables[dst->sa_family]) == 0)
+       if ((rnh = rt_tables[dst->sa_family]) == NULL)
                senderr(ESRCH);
        if (flags & RTF_HOST)
-               netmask = 0;
+               netmask = NULL;
        switch (req) {
        case RTM_DELETE:
                if (netmask) {
                        rt_maskedcopy(dst, (struct sockaddr *)&deldst, netmask);
                        dst = (struct sockaddr *)&deldst;
                }
-               if ((rn = rnh->rnh_lookup(dst, netmask, rnh)) == 0)
+               if ((rn = rnh->rnh_lookup(dst, netmask, rnh)) == NULL)
                        senderr(ESRCH);
                rt = (struct rtentry *)rn;
                if ((rt->rt_flags & RTF_CLONING) != 0) {
                        /* clean up any cloned children */
                        rtflushclone(rnh, rt);
                }
-               if ((rn = rnh->rnh_deladdr(dst, netmask, rnh)) == 0)
+               if ((rn = rnh->rnh_deladdr(dst, netmask, rnh)) == NULL)
                        senderr(ESRCH);
                if (rn->rn_flags & (RNF_ACTIVE | RNF_ROOT))
                        panic ("rtrequest delete");
                rt = (struct rtentry *)rn;
                if (rt->rt_gwroute) {
-                       rt = rt->rt_gwroute; RTFREE(rt);
-                       (rt = (struct rtentry *)rn)->rt_gwroute = 0;
+                       RTFREE(rt->rt_gwroute);
+                       rt->rt_gwroute = NULL;
                }
                if (rt->rt_parent) {
                        rt->rt_parent->rt_refcnt--;
@@ -597,7 +598,7 @@
                break;

        case RTM_RESOLVE:
-               if (ret_nrt == 0 || (rt = *ret_nrt) == 0)
+               if (ret_nrt == NULL || (rt = *ret_nrt) == NULL)
                        senderr(EINVAL);
                if ((rt->rt_flags & RTF_CLONING) == 0)
                        senderr(EINVAL);
@@ -605,17 +606,17 @@
                flags = rt->rt_flags & ~(RTF_CLONING | RTF_STATIC);
                flags |= RTF_CLONED;
                gateway = rt->rt_gateway;
-               if ((netmask = rt->rt_genmask) == 0)
+               if ((netmask = rt->rt_genmask) == NULL)
                        flags |= RTF_HOST;
                goto makeroute;

        case RTM_ADD:
-               if (info->rti_ifa == 0 && (error = rt_getifa(info)))
+               if (info->rti_ifa == NULL && (error = rt_getifa(info)))
                        senderr(error);
                ifa = info->rti_ifa;
        makeroute:
                rt = pool_get(&rtentry_pool, PR_NOWAIT);
-               if (rt == 0)
+               if (rt == NULL)
                        senderr(ENOBUFS);
                Bzero(rt, sizeof(*rt));
                rt->rt_flags = RTF_UP | flags;
@@ -647,7 +648,7 @@
                        }
                        RTFREE(crt);
                }
-               if (rn == 0) {
+               if (rn == NULL) {
                        IFAFREE(ifa);
                        if ((rt->rt_flags & RTF_CLONED) != 0 && rt->rt_parent)
                                rtfree(rt->rt_parent);
@@ -674,13 +675,6 @@
        return (error);
 }

-#undef dst
-#undef gateway
-#undef netmask
-#undef ifaaddr
-#undef ifpaddr
-#undef flags
-
 int
 rt_setgate( struct rtentry *rt0, const struct sockaddr *dst,
        const struct sockaddr *gate)
@@ -689,16 +683,16 @@
        u_int dlen = ROUNDUP(dst->sa_len), glen = ROUNDUP(gate->sa_len);
        struct rtentry *rt = rt0;

-       if (rt->rt_gateway == 0 || glen > ROUNDUP(rt->rt_gateway->sa_len)) {
+       if (rt->rt_gateway == NULL || glen > ROUNDUP(rt->rt_gateway->sa_len)) {
                old = (caddr_t)rt_key(rt);
                R_Malloc(new, caddr_t, dlen + glen);
-               if (new == 0)
+               if (new == NULL)
                        return 1;
                Bzero(new, dlen + glen);
                rt->rt_nodes->rn_key = new;
        } else {
                new = __UNCONST(rt->rt_nodes->rn_key); /*XXXUNCONST*/
-               old = 0;
+               old = NULL;
        }
        Bcopy(gate, (rt->rt_gateway = (struct sockaddr *)(new + dlen)), glen);
        if (old) {
@@ -706,8 +700,8 @@
                Free(old);
        }
        if (rt->rt_gwroute) {
-               rt = rt->rt_gwroute; RTFREE(rt);
-               rt = rt0; rt->rt_gwroute = 0;
+               RTFREE(rt->rt_gwroute);
+               rt->rt_gwroute = NULL;
        }
        if (rt->rt_flags & RTF_GATEWAY) {
                rt->rt_gwroute = rtalloc1(gate, 1);
@@ -758,7 +752,7 @@
        struct rtentry *rt;
        struct sockaddr *dst, *odst;
        struct sockaddr_storage deldst;
-       struct rtentry *nrt = 0;
+       struct rtentry *nrt = NULL;
        int error;
        struct rt_addrinfo info;