encoding/binary: make type error more specific

Right now it says 'invalid type S' for a struct type S.
Instead, say which type inside the struct is the problem.

Fixes #4825.

R=golang-dev, iant
CC=golang-dev
https://golang.org/cl/7301102

diff --git a/src/pkg/encoding/binary/binary.go b/src/pkg/encoding/binary/binary.go
index 04d5723c1e27c0d8a0d7eba95b275f258d4fdb1d..edbac197d6440fd5b622ce3a6b641ccd448eb1d1 100644 (file)
--- a/src/pkg/encoding/binary/binary.go
+++ b/src/pkg/encoding/binary/binary.go
@@ -167,9 +167,9 @@ func Read(r io.Reader, order ByteOrder, data interface{}) error {
        default:
                return errors.New("binary.Read: invalid type " + d.Type().String())
        }
-       size := dataSize(v)
-       if size < 0 {
-               return errors.New("binary.Read: invalid type " + v.Type().String())
+       size, err := dataSize(v)
+       if err != nil {
+               return errors.New("binary.Read: " + err.Error())
        }
        d := &decoder{order: order, buf: make([]byte, size)}
        if _, err := io.ReadFull(r, d.buf); err != nil {
@@ -247,64 +247,68 @@ func Write(w io.Writer, order ByteOrder, data interface{}) error {
 
        // Fallback to reflect-based encoding.
        v := reflect.Indirect(reflect.ValueOf(data))
-       size := dataSize(v)
-       if size < 0 {
-               return errors.New("binary.Write: invalid type " + v.Type().String())
+       size, err := dataSize(v)
+       if err != nil {
+               return errors.New("binary.Write: " + err.Error())
        }
        buf := make([]byte, size)
        e := &encoder{order: order, buf: buf}
        e.value(v)
-       _, err := w.Write(buf)
+       _, err = w.Write(buf)
        return err
 }
 
 // Size returns how many bytes Write would generate to encode the value v, which
 // must be a fixed-size value or a slice of fixed-size values, or a pointer to such data.
 func Size(v interface{}) int {
-       return dataSize(reflect.Indirect(reflect.ValueOf(v)))
+       n, err := dataSize(reflect.Indirect(reflect.ValueOf(v)))
+       if err != nil {
+               return -1
+       }
+       return n
 }
 
 // dataSize returns the number of bytes the actual data represented by v occupies in memory.
 // For compound structures, it sums the sizes of the elements. Thus, for instance, for a slice
 // it returns the length of the slice times the element size and does not count the memory
 // occupied by the header.
-func dataSize(v reflect.Value) int {
+func dataSize(v reflect.Value) (int, error) {
        if v.Kind() == reflect.Slice {
-               elem := sizeof(v.Type().Elem())
-               if elem < 0 {
-                       return -1
+               elem, err := sizeof(v.Type().Elem())
+               if err != nil {
+                       return 0, err
                }
-               return v.Len() * elem
+               return v.Len() * elem, nil
        }
        return sizeof(v.Type())
 }
 
-func sizeof(t reflect.Type) int {
+func sizeof(t reflect.Type) (int, error) {
        switch t.Kind() {
        case reflect.Array:
-               n := sizeof(t.Elem())
-               if n < 0 {
-                       return -1
+               n, err := sizeof(t.Elem())
+               if err != nil {
+                       return 0, err
                }
-               return t.Len() * n
+               return t.Len() * n, nil
 
        case reflect.Struct:
                sum := 0
                for i, n := 0, t.NumField(); i < n; i++ {
-                       s := sizeof(t.Field(i).Type)
-                       if s < 0 {
-                               return -1
+                       s, err := sizeof(t.Field(i).Type)
+                       if err != nil {
+                               return 0, err
                        }
                        sum += s
                }
-               return sum
+               return sum, nil
 
        case reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64,
                reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
                reflect.Float32, reflect.Float64, reflect.Complex64, reflect.Complex128:
-               return int(t.Size())
+               return int(t.Size()), nil
        }
-       return -1
+       return 0, errors.New("invalid type " + t.String())
 }
 
 type coder struct {
@@ -514,11 +518,12 @@ func (e *encoder) value(v reflect.Value) {
 }
 
 func (d *decoder) skip(v reflect.Value) {
-       d.buf = d.buf[dataSize(v):]
+       n, _ := dataSize(v)
+       d.buf = d.buf[n:]
 }
 
 func (e *encoder) skip(v reflect.Value) {
-       n := dataSize(v)
+       n, _ := dataSize(v)
        for i := range e.buf[0:n] {
                e.buf[i] = 0
        }

diff --git a/src/pkg/encoding/binary/binary_test.go b/src/pkg/encoding/binary/binary_test.go
index cfad8d36c7be1b0b44884df9f886b87b83bc8da3..056f0998f27b94c01232f370360c65b65eab3636 100644 (file)
--- a/src/pkg/encoding/binary/binary_test.go
+++ b/src/pkg/encoding/binary/binary_test.go
@@ -9,6 +9,7 @@ import (
        "io"
        "math"
        "reflect"
+       "strings"
        "testing"
 )
 
@@ -149,8 +150,14 @@ func TestWriteT(t *testing.T) {
 
        tv := reflect.Indirect(reflect.ValueOf(ts))
        for i, n := 0, tv.NumField(); i < n; i++ {
+               typ := tv.Field(i).Type().String()
+               if typ == "[4]int" {
+                       typ = "int" // the problem is int, not the [4]
+               }
                if err := Write(buf, BigEndian, tv.Field(i).Interface()); err == nil {
                        t.Errorf("WriteT.%v: have err == nil, want non-nil", tv.Field(i).Type())
+               } else if !strings.Contains(err.Error(), typ) {
+                       t.Errorf("WriteT: have err == %q, want it to mention %s", err, typ)
                }
        }
 }
@@ -238,7 +245,7 @@ func BenchmarkReadStruct(b *testing.B) {
        bsr := &byteSliceReader{}
        var buf bytes.Buffer
        Write(&buf, BigEndian, &s)
-       n := dataSize(reflect.ValueOf(s))
+       n, _ := dataSize(reflect.ValueOf(s))
        b.SetBytes(int64(n))
        t := s
        b.ResetTimer()