Re: [PATCH] IPv6: Miscellaneous clean-ups

Arnaldo Carvalho de Melo (acme@conectiva.com.br)
Fri, 4 Oct 2002 02:09:23 -0300


Em Fri, Oct 04, 2002 at 07:36:42AM +0900, YOSHIFUJI Hideaki / ?$B5HF#1QL@ escreveu:
> In article <20021003.103617.04446177.davem@redhat.com> (at Thu, 03 Oct 2002 10:36:17 -0700 (PDT)), "David S. Miller" <davem@redhat.com> says:
>
> > - addr.s6_addr[15] = 1;
> > + addr.s6_addr32[3] = __constant_htonl(0x00000001);
> >
> > Do not use __constant_htonl() in runtime code, use htonl().
> > Arnaldo de Melo told you this the other day for another one
> > of your patches, so you must fix this kind of stuff up before
> > I'll apply any of your patches which have this problem.
>
> I saw many __constant_{hton,ntoh}{s,l}()s, so fixed.
>
> 1. use s6_addrXX instead of in6_u.s6_addrXX.
> 2. avoid using magic number.
> 3. use 32bit constants.
> --> 4. avoid __constant_{hton,ntoh}{l,s}() in runtime code.

Thank you, some still were left, but that is not a problem, we can go on fixing
it, as long as we don't introduce new instances, its OK.

> ===================================================================
> RCS file: /cvsroot/usagi/usagi-backport/linux24/net/ipv6/addrconf.c,v
> retrieving revision 1.1.1.1
> diff -u -r1.1.1.1 addrconf.c
> --- net/ipv6/addrconf.c 2002/08/20 09:47:02 1.1.1.1
> +++ net/ipv6/addrconf.c 2002/10/03 22:16:13
> @@ -172,7 +172,7 @@
>
> if ((addr->s6_addr32[0] | addr->s6_addr32[1]) == 0) {
> if (addr->s6_addr32[2] == 0) {
> - if (addr->in6_u.u6_addr32[3] == 0)
> + if (addr->s6_addr32[3] == 0)
> return IPV6_ADDR_ANY;
>
> if (addr->s6_addr32[3] == __constant_htonl(0x00000001))
^^^^^^^^^^^
^^^^^^^^^^^
not needed as well.

> Index: net/ipv6/icmp.c
> ===================================================================
> RCS file: /cvsroot/usagi/usagi-backport/linux24/net/ipv6/icmp.c,v
> retrieving revision 1.1.1.1
> retrieving revision 1.1.1.1.12.1
> diff -u -r1.1.1.1 -r1.1.1.1.12.1
> --- net/ipv6/icmp.c 2002/08/20 09:47:02 1.1.1.1
> +++ net/ipv6/icmp.c 2002/09/12 09:41:58 1.1.1.1.12.1
> @@ -198,7 +198,7 @@
> u8 type;
> if (skb_copy_bits(skb, ptr+offsetof(struct icmp6hdr, icmp6_type),
> &type, 1)
> - || !(type & 0x80))
> + || !(type & ICMPV6_INFOMSG_MASK))

nice, no magic numbers.

> RCS file: /cvsroot/usagi/usagi-backport/linux24/net/ipv6/netfilter/ip6_queue.c,v
> retrieving revision 1.1.1.1
> retrieving revision 1.1.1.1.12.2
> diff -u -r1.1.1.1 -r1.1.1.1.12.2
> --- net/ipv6/netfilter/ip6_queue.c 2002/08/20 09:47:02 1.1.1.1
> +++ net/ipv6/netfilter/ip6_queue.c 2002/09/19 03:57:51 1.1.1.1.12.2
> @@ -306,14 +306,8 @@
> */
> if (e->info->hook == NF_IP_LOCAL_OUT) {
> struct ipv6hdr *iph = e->skb->nh.ipv6h;
> - if (!( iph->daddr.in6_u.u6_addr32[0] == e->rt_info.daddr.in6_u.u6_addr32[0]
> - && iph->daddr.in6_u.u6_addr32[1] == e->rt_info.daddr.in6_u.u6_addr32[1]
> - && iph->daddr.in6_u.u6_addr32[2] == e->rt_info.daddr.in6_u.u6_addr32[2]
> - && iph->daddr.in6_u.u6_addr32[3] == e->rt_info.daddr.in6_u.u6_addr32[3]
> - && iph->saddr.in6_u.u6_addr32[0] == e->rt_info.saddr.in6_u.u6_addr32[0]
> - && iph->saddr.in6_u.u6_addr32[1] == e->rt_info.saddr.in6_u.u6_addr32[1]
> - && iph->saddr.in6_u.u6_addr32[2] == e->rt_info.saddr.in6_u.u6_addr32[2]
> - && iph->saddr.in6_u.u6_addr32[3] == e->rt_info.saddr.in6_u.u6_addr32[3]))
> + if (ipv6_addr_cmp(&iph->daddr, &e->rt_info.daddr) ||
> + ipv6_addr_cmp(&iph->saddr, &e->rt_info.saddr))

Cool, thank you.

- Arnaldo
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/