From: Brad Fitzpatrick Date: Sun, 10 Nov 2019 19:59:00 +0000 (+0000) Subject: Revert "net: halve the allocs in ParseCIDR by sharing slice backing" X-Git-Tag: go1.14beta1~272 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=47bc24091af46408814d4e4b3a596fa720c7fd80;p=gostls13.git Revert "net: halve the allocs in ParseCIDR by sharing slice backing" This reverts CL 129118 (commit aff3aaa47f16d69efc50b6fec0ddc938176695eb) Reason for revert: It was retracted by the author in a comment on the PR but that doesn't get synced to Gerrit, and the Gerrit CL wasn't closed when the PR was closed. Change-Id: I5ad16e96f98a927972187dc5c9df3a0e9b9fafa8 Reviewed-on: https://go-review.googlesource.com/c/go/+/206377 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Emmanuel Odeke --- diff --git a/src/net/ip.go b/src/net/ip.go index 407337d9b3..9d1223e705 100644 --- a/src/net/ip.go +++ b/src/net/ip.go @@ -78,20 +78,10 @@ func CIDRMask(ones, bits int) IPMask { if ones < 0 || ones > bits { return nil } - return putCIDRMask(nil, ones, bits/8) -} - -// putCIDRMask will put an IPMask into `m` consisting of `ones` 1 bits -// followed by 0s up to a total length of `bits' bits if the length -// of m is < `l` bytes and returns the same slice m. If m did not have -// sufficient length for the mask l a new m is returned instead. -func putCIDRMask(m IPMask, ones, bits int) IPMask { - if len(m) < bits { - m = make(IPMask, bits) - } - + l := bits / 8 + m := make(IPMask, l) n := uint(ones) - for i := 0; i < bits; i++ { + for i := 0; i < l; i++ { if n >= 8 { m[i] = 0xff n -= 8 @@ -256,13 +246,6 @@ func allFF(b []byte) bool { // Mask returns the result of masking the IP address ip with mask. func (ip IP) Mask(mask IPMask) IP { - return ipMaskWithBuf(nil, ip, mask) -} - -// ipMaskWithBuf implements the IP.Mask method, but containing an -// opitional optional bufer to use for the return value. If the buf is -// too small, a new one is allocated. -func ipMaskWithBuf(buf, ip IP, mask IPMask) []byte { if len(mask) == IPv6len && len(ip) == IPv4len && allFF(mask[:12]) { mask = mask[12:] } @@ -273,13 +256,11 @@ func ipMaskWithBuf(buf, ip IP, mask IPMask) []byte { if n != len(mask) { return nil } - if len(buf) < n { - buf = make(IP, n) - } + out := make(IP, n) for i := 0; i < n; i++ { - buf[i] = ip[i] & mask[i] + out[i] = ip[i] & mask[i] } - return buf + return out } // ubtoa encodes the string form of the integer v to dst[start:] and @@ -556,35 +537,28 @@ func (n *IPNet) String() string { // Parse IPv4 address (d.d.d.d). func parseIPv4(s string) IP { var p [IPv4len]byte - if !parseIntoIPv4(p[:], s) { - return nil - } - return IPv4(p[0], p[1], p[2], p[3]) -} - -// parseIntoIPv4 parses s as an IPv4 address and parses it into -// p, which must be at least IPv4len bytes long. It reports -// whether the parse was successful. -func parseIntoIPv4(p []byte, s string) bool { for i := 0; i < IPv4len; i++ { if len(s) == 0 { // Missing octets. - return false + return nil } if i > 0 { if s[0] != '.' { - return false + return nil } s = s[1:] } n, c, ok := dtoi(s) if !ok || n > 0xFF { - return false + return nil } s = s[c:] p[i] = byte(n) } - return len(s) == 0 + if len(s) != 0 { + return nil + } + return IPv4(p[0], p[1], p[2], p[3]) } // parseIPv6Zone parses s as a literal IPv6 address and its associated zone @@ -597,16 +571,7 @@ func parseIPv6Zone(s string) (IP, string) { // parseIPv6 parses s as a literal IPv6 address described in RFC 4291 // and RFC 5952. func parseIPv6(s string) (ip IP) { - if ip = make(IP, IPv6len); !parseIntoIPv6(ip, s) { - ip = nil - } - return -} - -// parseIntoIPv6 parses s as a literal IPv6 address described in RFC -// 4291 and RFC 5952 and populates ip. It reports whether the parse -// was successful. -func parseIntoIPv6(ip IP, s string) bool { + ip = make(IP, IPv6len) ellipsis := -1 // position of ellipsis in ip // Might have leading ellipsis @@ -615,7 +580,7 @@ func parseIntoIPv6(ip IP, s string) bool { s = s[2:] // Might be only ellipsis if len(s) == 0 { - return true + return ip } } @@ -625,22 +590,22 @@ func parseIntoIPv6(ip IP, s string) bool { // Hex number. n, c, ok := xtoi(s) if !ok || n > 0xFFFF { - return false + return nil } // If followed by dot, might be in trailing IPv4. if c < len(s) && s[c] == '.' { if ellipsis < 0 && i != IPv6len-IPv4len { // Not the right place. - return false + return nil } if i+IPv4len > IPv6len { // Not enough room. - return false + return nil } ip4 := parseIPv4(s) if ip4 == nil { - return false + return nil } ip[i] = ip4[12] ip[i+1] = ip4[13] @@ -664,14 +629,14 @@ func parseIntoIPv6(ip IP, s string) bool { // Otherwise must be followed by colon and more. if s[0] != ':' || len(s) == 1 { - return false + return nil } s = s[1:] // Look for ellipsis. if s[0] == ':' { if ellipsis >= 0 { // already have one - return false + return nil } ellipsis = i s = s[1:] @@ -683,13 +648,13 @@ func parseIntoIPv6(ip IP, s string) bool { // Must have used entire string. if len(s) != 0 { - return false + return nil } // If didn't parse enough, expand ellipsis. if i < IPv6len { if ellipsis < 0 { - return false + return nil } n := IPv6len - i for j := i - 1; j >= ellipsis; j-- { @@ -700,9 +665,9 @@ func parseIntoIPv6(ip IP, s string) bool { } } else if ellipsis >= 0 { // Ellipsis must represent at least one 0 group. - return false + return nil } - return true + return ip } // ParseIP parses s as an IP address, returning the result. @@ -711,29 +676,15 @@ func parseIntoIPv6(ip IP, s string) bool { // If s is not a valid textual representation of an IP address, // ParseIP returns nil. func ParseIP(s string) IP { - switch n := strToIPvLen(s); n { - case IPv4len: - return parseIPv4(s) - case IPv6len: - return parseIPv6(s) - default: - return nil - } -} - -// strToIPvLen reports the expected length of the parsed IP adress -// given its ASCII representation in s. It returns IPv4len, IPv6len, -// or -1. -func strToIPvLen(s string) int { for i := 0; i < len(s); i++ { switch s[i] { case '.': - return IPv4len + return parseIPv4(s) case ':': - return IPv6len + return parseIPv6(s) } } - return -1 + return nil } // parseIPZone parses s as an IP address, return it and its associated zone @@ -763,53 +714,17 @@ func ParseCIDR(s string) (IP, *IPNet, error) { if i < 0 { return nil, nil, &ParseError{Type: "CIDR address", Text: s} } - - iplen := strToIPvLen(s[:i]) - if iplen < 0 { - return nil, nil, &ParseError{Type: "CIDR address", Text: s} + addr, mask := s[:i], s[i+1:] + iplen := IPv4len + ip := parseIPv4(addr) + if ip == nil { + iplen = IPv6len + ip = parseIPv6(addr) } - - nwlen, y, ok := dtoi(s[i+1:]) - if !ok || y != len(s[i+1:]) || nwlen < 0 || nwlen > 8*iplen { + n, i, ok := dtoi(mask) + if ip == nil || !ok || i != len(mask) || n < 0 || n > 8*iplen { return nil, nil, &ParseError{Type: "CIDR address", Text: s} } - - var ( - buf []byte - ipn *IPNet - ) - switch iplen { - case IPv4len: - var block struct { - ipn IPNet - arr [IPv6len + (IPv4len * 2)]byte - } - ipn = &block.ipn - buf = block.arr[:] - copy(buf, v4InV6Prefix) - if !parseIntoIPv4(buf[len(v4InV6Prefix):], s[:i]) { - return nil, nil, &ParseError{Type: "CIDR address", Text: s} - } - case IPv6len: - var block struct { - ipn IPNet - arr [IPv6len * 3]byte - } - ipn = &block.ipn - buf = block.arr[:] - if !parseIntoIPv6(buf, s[:i]) { - return nil, nil, &ParseError{Type: "CIDR address", Text: s} - } - default: - return nil, nil, &ParseError{Type: "CIDR address", Text: s} - } - - ip := buf[:IPv6len:IPv6len] - buf = buf[IPv6len:] - - ipn.Mask = putCIDRMask(buf[:iplen:iplen], nwlen, iplen) - buf = buf[iplen:] - - ipn.IP = ipMaskWithBuf(buf[:iplen:iplen], ip, ipn.Mask) - return ip, ipn, nil + m := CIDRMask(n, 8*iplen) + return ip, &IPNet{IP: ip.Mask(m), Mask: m}, nil } diff --git a/src/net/ip_test.go b/src/net/ip_test.go index 4e7f0cf01f..a5fc5e644a 100644 --- a/src/net/ip_test.go +++ b/src/net/ip_test.go @@ -46,18 +46,14 @@ var parseIPTests = []struct { func TestParseIP(t *testing.T) { for _, tt := range parseIPTests { - out := ParseIP(tt.in) - if !reflect.DeepEqual(out, tt.out) { + if out := ParseIP(tt.in); !reflect.DeepEqual(out, tt.out) { t.Errorf("ParseIP(%q) = %v, want %v", tt.in, out, tt.out) } - if exp, got := cap(out), len(out); exp != got { - t.Fatalf("ParseIP(%q) cap %v; want %v", tt.in, exp, got) - } if tt.in == "" { // Tested in TestMarshalEmptyIP below. continue } - out = nil + var out IP if err := out.UnmarshalText([]byte(tt.in)); !reflect.DeepEqual(out, tt.out) || (tt.out == nil) != (err != nil) { t.Errorf("IP.UnmarshalText(%q) = %v, %v, want %v", tt.in, out, err, tt.out) } @@ -111,7 +107,6 @@ func BenchmarkParseIP(b *testing.B) { testHookUninstaller.Do(uninstallTestHooks) for i := 0; i < b.N; i++ { - b.ReportAllocs() for _, tt := range parseIPTests { ParseIP(tt.in) } @@ -371,41 +366,12 @@ func TestParseCIDR(t *testing.T) { if !reflect.DeepEqual(err, tt.err) { t.Errorf("ParseCIDR(%q) = %v, %v; want %v, %v", tt.in, ip, net, tt.ip, tt.net) } - if err != nil { - continue - } - if !tt.ip.Equal(ip) || !tt.net.IP.Equal(net.IP) || !reflect.DeepEqual(net.Mask, tt.net.Mask) { + if err == nil && (!tt.ip.Equal(ip) || !tt.net.IP.Equal(net.IP) || !reflect.DeepEqual(net.Mask, tt.net.Mask)) { t.Errorf("ParseCIDR(%q) = %v, {%v, %v}; want %v, {%v, %v}", tt.in, ip, net.IP, net.Mask, tt.ip, tt.net.IP, tt.net.Mask) } - if exp, got := cap(ip), len(ip); exp != got { - t.Fatalf("ParseCIDR(%q) ip cap %v; want %v", tt.in, exp, got) - } - if exp, got := cap(net.IP), len(net.IP); exp != got { - t.Fatalf("ParseCIDR(%q) net.IP cap %v; want %v", tt.in, exp, got) - } - if exp, got := cap(net.Mask), len(net.Mask); exp != got { - t.Fatalf("ParseCIDR(%q) net.Mask cap %v; want %v", tt.in, exp, got) - } } } -func BenchmarkParseCIDR(b *testing.B) { - testHookUninstaller.Do(uninstallTestHooks) - - b.Run("IPv4", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - ParseCIDR("135.104.0.1/24") - } - }) - b.Run("IPv6", func(b *testing.B) { - b.ReportAllocs() - for i := 0; i < b.N; i++ { - ParseCIDR("2001:DB8::1/48") - } - }) -} - var ipNetContainsTests = []struct { ip IP net *IPNet