Correctly decode struct values

guelfey · guelfey · commit 2ba8c5612976 · 2013-07-25T16:10:26.000+02:00
For message decoding, the signature of the message is now considered
to properly decode structs into []interface{}, and Store converts
anything containing such values to respective Go structs. This
also removes the need for Signature.Values(), which is removed with
this commit.

Fixes #18.
diff --git a/dbus.go b/dbus.go
@@ -45,20 +45,32 @@ func Store(src []interface{}, dest ...interface{}) error {
 		return errors.New("dbus.Store: length mismatch")
 	}
 
-	for i, v := range src {
-		if reflect.TypeOf(dest[i]).Elem() == reflect.TypeOf(v) {
-			reflect.ValueOf(dest[i]).Elem().Set(reflect.ValueOf(v))
-		} else if vs, ok := v.([]interface{}); ok {
-			retv := reflect.ValueOf(dest[i]).Elem()
-			if retv.Kind() != reflect.Struct {
+	for i := range src {
+		if err := store(src[i], dest[i]); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func store(src, dest interface{}) error {
+	if reflect.TypeOf(dest).Elem() == reflect.TypeOf(src) {
+		reflect.ValueOf(dest).Elem().Set(reflect.ValueOf(src))
+		return nil
+	} else if hasStruct(dest) {
+		rv := reflect.ValueOf(dest).Elem()
+		switch rv.Kind() {
+		case reflect.Struct:
+			vs, ok := src.([]interface{})
+			if !ok {
 				return errors.New("dbus.Store: type mismatch")
 			}
-			t := retv.Type()
-			ndest := make([]interface{}, 0, retv.NumField())
-			for i := 0; i < retv.NumField(); i++ {
+			t := rv.Type()
+			ndest := make([]interface{}, 0, rv.NumField())
+			for i := 0; i < rv.NumField(); i++ {
 				field := t.Field(i)
 				if field.PkgPath == "" && field.Tag.Get("dbus") != "-" {
-					ndest = append(ndest, retv.Field(i).Addr().Interface())
+					ndest = append(ndest, rv.Field(i).Addr().Interface())
 				}
 			}
 			if len(vs) != len(ndest) {
@@ -68,11 +80,52 @@ func Store(src []interface{}, dest ...interface{}) error {
 			if err != nil {
 				return errors.New("dbus.Store: type mismatch")
 			}
-		} else {
+		case reflect.Slice:
+			sv := reflect.ValueOf(src)
+			if sv.Kind() != reflect.Slice {
+				return errors.New("dbus.Store: type mismatch")
+			}
+			rv.Set(reflect.MakeSlice(rv.Type(), sv.Len(), sv.Len()))
+			for i := 0; i < sv.Len(); i++ {
+				if err := store(sv.Index(i).Interface(), rv.Index(i).Addr().Interface()); err != nil {
+					return err
+				}
+			}
+		case reflect.Map:
+			sv := reflect.ValueOf(src)
+			if sv.Kind() != reflect.Map {
+				return errors.New("dbus.Store: type mismatch")
+			}
+			keys := sv.MapKeys()
+			rv.Set(reflect.MakeMap(sv.Type()))
+			for _, key := range keys {
+				v := reflect.New(sv.Type().Elem())
+				if err := store(v, sv.MapIndex(key).Interface()); err != nil {
+					return err
+				}
+				rv.SetMapIndex(key, v.Elem())
+			}
+		default:
 			return errors.New("dbus.Store: type mismatch")
 		}
+		return nil
+	} else {
+		return errors.New("dbus.Store: type mismatch")
+	}
+}
+
+func hasStruct(v interface{}) bool {
+	t := reflect.TypeOf(v)
+	for {
+		switch t.Kind() {
+		case reflect.Struct:
+			return true
+		case reflect.Slice, reflect.Ptr, reflect.Map:
+			t = t.Elem()
+		default:
+			return false
+		}
 	}
-	return nil
 }
 
 // An ObjectPath is an object path as defined by the D-Bus spec.
diff --git a/decoder.go b/decoder.go
@@ -6,17 +6,6 @@ import (
 	"reflect"
 )
 
-// A decoder reads values that are encoded in the D-Bus wire format.
-//
-// For decoding, the inverse of the encoding that an Encoder applies is used,
-// with the following exceptions:
-//
-//     - If a VARIANT contains a STRUCT, its decoded value will be of type
-//       []interface{} and contain the struct fields in the correct order.
-//     - When decoding into a pointer, the pointer is followed unless it is nil,
-//       in which case a new value for it to point to is allocated.
-//     - When decoding into a slice, the decoded values are appended to it.
-//     - Arrays cannot be decoded into.
 type decoder struct {
 	in    io.Reader
 	order binary.ByteOrder
@@ -71,6 +60,86 @@ func (dec *decoder) Decode(vs ...interface{}) (err error) {
 	return nil
 }
 
+func (dec *decoder) DecodeSig(sig Signature) (vs []interface{}, err error) {
+	defer func() {
+		var ok bool
+		v := recover()
+		if err, ok = v.(error); ok {
+			if err == io.EOF || err == io.ErrUnexpectedEOF {
+				err = FormatError("unexpected EOF")
+			}
+		}
+	}()
+	vs = make([]interface{}, 0)
+	s := sig.str
+	for s != "" {
+		err, rem := validSingle(s, 0)
+		if err != nil {
+			return nil, err
+		}
+		v := dec.decodeSig(s[:len(s)-len(rem)], 0)
+		vs = append(vs, v)
+		s = rem
+	}
+	return vs, nil
+}
+
+func (dec *decoder) decodeSig(s string, depth int) interface{} {
+	if len(s) == 1 {
+		if t, ok := sigToType[s[0]]; ok {
+			v := reflect.New(t)
+			dec.decode(v, depth)
+			return v.Elem().Interface()
+		}
+	}
+	switch s[0] {
+	case 'a':
+		if len(s) > 1 && s[1] == '{' {
+			ksig := s[2:3]
+			vsig := s[3 : len(s)-1]
+			v := reflect.MakeMap(reflect.MapOf(typeFor(ksig), typeFor(vsig)))
+			if depth >= 63 {
+				panic(FormatError("input exceeds container depth limit"))
+			}
+			var length uint32
+			dec.decode(reflect.ValueOf(&length), depth)
+			spos := dec.pos
+			for dec.pos < spos+int(length) {
+				dec.align(8)
+				if !isKeyType(v.Type().Key()) {
+					panic(InvalidTypeError{v.Type()})
+				}
+				kv := dec.decodeSig(ksig, depth+2)
+				vv := dec.decodeSig(vsig, depth+2)
+				v.SetMapIndex(reflect.ValueOf(kv), reflect.ValueOf(vv))
+			}
+			return v.Interface()
+		}
+		var length uint32
+		if depth >= 64 {
+			panic(FormatError("input exceeds container depth limit"))
+		}
+		dec.decode(reflect.ValueOf(&length), depth)
+		v := reflect.MakeSlice(reflect.SliceOf(typeFor(s[1:])), 0, int(length))
+		spos := dec.pos
+		for dec.pos < spos+int(length) {
+			ev := dec.decodeSig(s[1:], depth+1)
+			v = reflect.Append(v, reflect.ValueOf(ev))
+		}
+		return v.Interface()
+	case '(':
+		dec.align(8)
+		v := make([]interface{}, 0)
+		for _, c := range s[1 : len(s)-1] {
+			ev := dec.decodeSig(string(c), depth+1)
+			v = append(v, ev)
+		}
+		return v
+	default:
+		panic(SignatureError{Sig: s})
+	}
+}
+
 // decode decodes a single value and stores it in *v. depth holds the depth of
 // the container nesting.
 func (dec *decoder) decode(v reflect.Value, depth int) {
@@ -186,7 +255,7 @@ func (dec *decoder) decode(v reflect.Value, depth int) {
 			if rem != "" {
 				panic(FormatError("variant signature has multiple types"))
 			}
-			t = value(sig.str)
+			t = typeFor(sig.str)
 			if t == interfacesType {
 				dec.align(8)
 				s := sig.str[1 : len(sig.str)-1]
@@ -196,7 +265,7 @@ func (dec *decoder) decode(v reflect.Value, depth int) {
 					if err != nil {
 						panic(err)
 					}
-					t = value(s[:len(s)-len(rem)])
+					t = typeFor(s[:len(s)-len(rem)])
 					nv := reflect.New(t)
 					dec.decode(nv, depth+1)
 					slice = reflect.Append(slice, nv.Elem())
diff --git a/doc.go b/doc.go
@@ -44,17 +44,10 @@ Pointers encode as the value they're pointed to.
 Trying to encode any other type or a slice, map or struct containing an
 unsupported type will result in an InvalidTypeError.
 
-For incoming messages, the inverse of these rules are used, with the following
-exceptions:
-
-    - If an incoming STRUCT can't be directly converted to a Go struct (because
-      it is, for example, wrapped in a VARIANT), it is stored as a []interface{}
-      containing the struct fields in the correct order. The Store function can
-      be used to convert such values to the proper structs.
-    - When decoding into a pointer, the pointer is followed unless it is nil, in
-      which case a new value for it to point to is allocated.
-    - When decoding into a slice, the decoded values are appended to it.
-    - Arrays cannot be decoded into.
+For incoming messages, the inverse of these rules are used, with the exception
+of STRUCTs. Incoming STRUCTS are represented as a slice of empty interfaces
+containing the struct fields in the correct order. The Store function can be
+used to convert such values to Go structs.
 
 Unix FD passing
 
diff --git a/message.go b/message.go
@@ -167,13 +167,13 @@ func DecodeMessage(rd io.Reader) (msg *Message, err error) {
 	}
 	sig, _ := msg.Headers[FieldSignature].value.(Signature)
 	if sig.str != "" {
-		vs := sig.Values()
 		buf := bytes.NewBuffer(body)
 		dec = newDecoder(buf, order)
-		if err = dec.Decode(vs...); err != nil {
+		vs, err := dec.DecodeSig(sig)
+		if err != nil {
 			return nil, err
 		}
-		msg.Body = dereferenceAll(vs)
+		msg.Body = vs
 	}
 
 	return
diff --git a/proto_test.go b/proto_test.go
@@ -225,6 +225,17 @@ func TestMessage(t *testing.T) {
 	}
 }
 
+func TestProtoStructInterfaces(t *testing.T) {
+	b := []byte{42}
+	vs, err := newDecoder(bytes.NewReader(b), binary.LittleEndian).DecodeSig(Signature{"(y)"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if vs[0].([]interface{})[0].(byte) != 42 {
+		t.Errorf("wrongs results (got %v)", vs)
+	}
+}
+
 // ordinary org.freedesktop.DBus.Hello call
 var smallMessage = &Message{
 	Type:   TypeMethodCall,
diff --git a/sig.go b/sig.go
@@ -151,17 +151,6 @@ func (s Signature) String() string {
 	return s.str
 }
 
-// Values returns a slice of pointers to values that match the given signature.
-func (s Signature) Values() []interface{} {
-	slice := make([]interface{}, 0)
-	str := s.str
-	for str != "" {
-		slice = append(slice, reflect.New(value(str)).Interface())
-		_, str = validSingle(str, 0)
-	}
-	return slice
-}
-
 // A SignatureError indicates that a signature passed to a function or received
 // on a connection is not a valid signature.
 type SignatureError struct {
@@ -189,10 +178,11 @@ func validSingle(s string, depth int) (err error, rem string) {
 		return nil, s[1:]
 	case 'a':
 		if len(s) > 1 && s[1] == '{' {
-			i := strings.LastIndex(s, "}")
+			i := findMatching(s[1:], '{', '}')
 			if i == -1 {
 				return SignatureError{Sig: s, Reason: "unmatched '{'"}, ""
 			}
+			i++
 			rem = s[i+1:]
 			s = s[2:i]
 			if err, _ = validSingle(s[:1], depth+1); err != nil {
@@ -209,7 +199,7 @@ func validSingle(s string, depth int) (err error, rem string) {
 		}
 		return validSingle(s[1:], depth+1)
 	case '(':
-		i := strings.LastIndex(s, ")")
+		i := findMatching(s, '(', ')')
 		if i == -1 {
 			return SignatureError{Sig: s, Reason: "unmatched ')'"}, ""
 		}
@@ -226,9 +216,24 @@ func validSingle(s string, depth int) (err error, rem string) {
 	return SignatureError{Sig: s, Reason: "invalid type character"}, ""
 }
 
-// value returns the type of the given signature. It ignores any left over
+func findMatching(s string, left, right rune) int {
+	n := 0
+	for i, v := range s {
+		if v == left {
+			n++
+		} else if v == right {
+			n--
+		}
+		if n == 0 {
+			return i
+		}
+	}
+	return -1
+}
+
+// typeFor returns the type of the given signature. It ignores any left over
 // characters and panics if s doesn't start with a valid type signature.
-func value(s string) (t reflect.Type) {
+func typeFor(s string) (t reflect.Type) {
 	err, _ := validSingle(s, 0)
 	if err != nil {
 		panic(err)
@@ -241,9 +246,9 @@ func value(s string) (t reflect.Type) {
 	case 'a':
 		if s[1] == '{' {
 			i := strings.LastIndex(s, "}")
-			t = reflect.MapOf(sigToType[s[2]], value(s[3:i]))
+			t = reflect.MapOf(sigToType[s[2]], typeFor(s[3:i]))
 		} else {
-			t = reflect.SliceOf(value(s[1:]))
+			t = reflect.SliceOf(typeFor(s[1:]))
 		}
 	case '(':
 		t = interfacesType
diff --git a/sig_test.go b/sig_test.go

Original file line number	Diff line number	Diff line change
`@@ -167,13 +167,13 @@ func DecodeMessage(rd io.Reader) (msg *Message, err error) {`
`167`	`167`	`}`
`168`	`168`	`sig, _ := msg.Headers[FieldSignature].value.(Signature)`
`169`	`169`	`if sig.str != "" {`
`170`		`- vs := sig.Values()`
`171`	`170`	`buf := bytes.NewBuffer(body)`
`172`	`171`	`dec = newDecoder(buf, order)`
`173`		`- if err = dec.Decode(vs...); err != nil {`
	`172`	`+ vs, err := dec.DecodeSig(sig)`
	`173`	`+ if err != nil {`
`174`	`174`	`return nil, err`
`175`	`175`	`}`
`176`		`- msg.Body = dereferenceAll(vs)`
	`176`	`+ msg.Body = vs`
`177`	`177`	`}`
`178`	`178`
`179`	`179`	`return`