code review 438042: net: fix IPMask.String not to crash on all-0xff mask

net: fix IPMask.String not to crash on all-0xff mask

[rsc, 2010-03-13 01:48:49]

Hello r (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[r2, 2010-03-13 01:52:55]

LGTM

On Mar 12, 2010, at 5:48 PM, rsc@golang.org wrote:

> Reviewers: r,
>
> Message:
> Hello r (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change.
>
>
> Description:
> net: fix IPMask.String not to crash on all-0xff mask
>
> Please review this at http://codereview.appspot.com/438042/show
>
> Affected files:
>  M src/pkg/net/ip.go
>
>
> Index: src/pkg/net/ip.go
> ===================================================================
> --- a/src/pkg/net/ip.go
> +++ b/src/pkg/net/ip.go
> @@ -51,6 +51,20 @@
> 	return p
> }
>
> +// IPv4Mask returns the IP mask (in 16-byte form) of the
> +// IPv4 mask a.b.c.d.
> +func IPv4Mask(a, b, c, d byte) IPMask {
> +	p := make(IPMask, IPv6len)
> +	for i := 0; i < 12; i++ {
> +		p[i] = 0xff
> +	}
> +	p[12] = a
> +	p[13] = b
> +	p[14] = c
> +	p[15] = d
> +	return p
> +}
> +
> // Well-known IPv4 addresses
> var (
> 	IPv4bcast     = IPv4(255, 255, 255, 255) // broadcast
> @@ -103,9 +117,9 @@
>
> // Default route masks for IPv4.
> var (
> -	classAMask = IPMask(IPv4(0xff, 0, 0, 0))
> -	classBMask = IPMask(IPv4(0xff, 0xff, 0, 0))
> -	classCMask = IPMask(IPv4(0xff, 0xff, 0xff, 0))
> +	classAMask = IPv4Mask(0xff, 0, 0, 0)
> +	classBMask = IPv4Mask(0xff, 0xff, 0, 0)
> +	classCMask = IPv4Mask(0xff, 0xff, 0xff, 0)
> )
>
> // DefaultMask returns the default IP mask for the IP address ip.
> @@ -229,25 +243,28 @@
> // If mask is a sequence of 1 bits followed by 0 bits,
> // return the number of 1 bits.
> func simpleMaskLength(mask IPMask) int {
> -	var i int
> -	for i = 0; i < len(mask); i++ {
> -		if mask[i] != 0xFF {
> -			break
> +	var n int
> +	for i, v := range mask {
> +		if v == 0xff {
> +			n += 8
> +			continue
> 		}
> -	}
> -	n := 8 * i
> -	v := mask[i]
> -	for v&0x80 != 0 {
> -		n++
> -		v <<= 1
> -	}
> -	if v != 0 {
> -		return -1
> -	}
> -	for i++; i < len(mask); i++ {
> -		if mask[i] != 0 {
> +		// found non-ff byte
> +		// count 1 bits
> +		for v&0x80 != 0 {
> +			n++
> +			v <<= 1
> +		}
> +		// rest must be 0 bits
> +		if v != 0 {
> 			return -1
> 		}
> +		for i++; i < len(mask); i++ {
> +			if mask[i] != 0 {
> +				return -1
> +			}
> +		}
> +		break
> 	}
> 	return n
> }
> @@ -266,8 +283,8 @@
> 		}
> 	case 16:
> 		n := simpleMaskLength(mask)
> -		if n >= 0 {
> -			return itod(uint(n))
> +		if n >= 12*8 {
> +			return itod(uint(n - 12*8))
> 		}
> 	}
> 	return IP(mask).String()
>
>

[rsc, 2010-03-16 21:16:36]

*** Submitted as http://code.google.com/p/go/source/detail?r=7cb27ce45182 ***

net: fix IPMask.String not to crash on all-0xff mask

R=r
CC=golang-dev
http://codereview.appspot.com/438042