code review 12705043: net: make IP implement encoding.MarshalerText, encoding...

net: make IP implement encoding.MarshalerText, encoding.UnmarshalerText

See golang.org/s/go12encoding for design.

[rsc, 2013-08-09 19:31:11]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go/

[bradfitz, 2013-08-09 19:31:59]

LGTM



On Fri, Aug 9, 2013 at 12:31 PM, <rsc@golang.org> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> net: make IP implement encoding.MarshalerText, encoding.UnmarshalerText
>
> See golang.org/s/go12encoding for design.
>
> Please review this at
https://codereview.appspot.**com/12705043/<https://codereview.appspot.com/127...
>
> Affected files:
>   M src/pkg/net/ip.go
>   M src/pkg/net/ip_test.go
>
>
> Index: src/pkg/net/ip.go
> ==============================**==============================**=======
> --- a/src/pkg/net/ip.go
> +++ b/src/pkg/net/ip.go
> @@ -12,6 +12,8 @@
>
>  package net
>
> +import "errors"
> +
>  // IP address lengths (bytes).
>  const (
>         IPv4len = 4
> @@ -310,6 +312,27 @@
>         return s
>  }
>
> +// MarshalText implements the encoding.TextMarshaler interface.
> +// The encoding is the same as returned by String.
> +func (ip IP) MarshalText() ([]byte, error) {
> +       if len(ip) != IPv4len && len(ip) != IPv6len {
> +               return nil, errors.New("invalid IP address")
> +       }
> +       return []byte(ip.String()), nil
> +}
> +
> +// UnmarshalText implements the encoding.TextUnmarshaler interface.
> +// The IP address is expected in a form accepted by ParseIP.
> +func (ip *IP) UnmarshalText(text []byte) error {
> +       s := string(text)
> +       x := ParseIP(s)
> +       if x == nil {
> +               return &ParseError{"IP address", s}
> +       }
> +       *ip = x
> +       return nil
> +}
> +
>  // Equal returns true if ip and x are the same IP address.
>  // An IPv4 address and that same address in IPv6 form are
>  // considered to be equal.
> Index: src/pkg/net/ip_test.go
> ==============================**==============================**=======
> --- a/src/pkg/net/ip_test.go
> +++ b/src/pkg/net/ip_test.go
> @@ -32,6 +32,11 @@
>                 if out := ParseIP(tt.in); !reflect.DeepEqual(out, tt.out)
> {
>                         t.Errorf("ParseIP(%q) = %v, want %v", tt.in, out,
> tt.out)
>                 }
> +               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)
> +               }
>         }
>  }
>
> @@ -47,6 +52,7 @@
>         {IP{0x20, 0x1, 0xd, 0xb8, 0, 0, 0, 0, 0, 0x1, 0, 0, 0, 0, 0, 0},
> "2001:db8:0:0:1::"},
>         {IP{0x20, 0x1, 0xd, 0xb8, 0, 0, 0, 0, 0, 0x1, 0, 0, 0, 0, 0, 0x1},
> "2001:db8::1:0:0:1"},
>         {IP{0x20, 0x1, 0xD, 0xB8, 0, 0, 0, 0, 0, 0xA, 0, 0xB, 0, 0xC, 0,
> 0xD}, "2001:db8::a:b:c:d"},
> +       {IPv4(192, 168, 0, 1), "192.168.0.1"},
>         {nil, "<nil>"},
>  }
>
> @@ -55,6 +61,15 @@
>                 if out := tt.in.String(); out != tt.out {
>                         t.Errorf("IP.String(%v) = %q, want %q", tt.in,
> out, tt.out)
>                 }
> +               if tt.in != nil {
> +                       if out, err := tt.in.MarshalText(); string(out) !=
> tt.out || err != nil {
> +                               t.Errorf("IP.MarshalText(%v) = %q, %v,
> want %q, nil", out, err, tt.out)
> +                       }
> +               } else {
> +                       if _, err := tt.in.MarshalText(); err == nil {
> +                               t.Errorf("IP.MarshalText(nil) succeeded,
> want failure")
> +                       }
> +               }
>         }
>  }
>
>
>
> --
>
> ---You received this message because you are subscribed to the Google
> Groups "golang-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to
golang-dev+unsubscribe@**googlegroups.com<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>

[mikio, 2013-08-10 00:05:25]

Fixes issue 5086?

https://codereview.appspot.com/12705043/diff/3001/src/pkg/net/ip.go
File src/pkg/net/ip.go (right):

https://codereview.appspot.com/12705043/diff/3001/src/pkg/net/ip.go#newcode319
src/pkg/net/ip.go:319: return nil, errors.New("invalid IP address")
why not use InvalidAddrError?

[rsc, 2013-08-14 04:33:24]

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

net: make IP implement encoding.MarshalerText, encoding.UnmarshalerText

See golang.org/s/go12encoding for design.

R=golang-dev, bradfitz, mikioh.mikioh
CC=golang-dev
https://codereview.appspot.com/12705043