prop: copy initial values and store as pointers internally

Without using pointers internally, accepting structs using Store
wouldn't work properly. This requires making a copy of the initial
values (which were documented already as "initial" values, but were
indeed just reused by the existing code). But using them directly was
anyway not possible since access not going through Get or GetMust could
have led to race conditions anyway.

Fixes #298.

prop/prop.go

@@ -126,7 +126,9 @@ const IntrospectDataString = `
 // Prop represents a single property. It is used for creating a Properties
 // value.
 type Prop struct {
-	// Initial value. Must be a DBus-representable type.
+	// Initial value. Must be a DBus-representable type. This is not modified
+	// after Properties has been initialized; use Get or GetMust to access the
+	// value.
 	Value interface{}
 
 	// If true, the value can be modified by calls to Set.
@@ -154,7 +156,7 @@ func (p *Prop) Introspection(name string) introspect.Property {
 	}
 	result.Annotations = []introspect.Annotation{
 		{
-			Name: "org.freedesktop.DBus.Property.EmitsChangedSignal",
+			Name:  "org.freedesktop.DBus.Property.EmitsChangedSignal",
 			Value: p.Emit.String(),
 		},
 	}
@@ -173,7 +175,7 @@ type Change struct {
 // using the org.freedesktop.DBus.Properties interface. It is safe for
 // concurrent use by multiple goroutines.
 type Properties struct {
-	m    map[string]map[string]*Prop
+	m    Map
 	mut  sync.RWMutex
 	conn *dbus.Conn
 	path dbus.ObjectPath
@@ -183,7 +185,7 @@ type Properties struct {
 // swallowing the error, shouldn't be used.
 //
 // Deprecated: use Export instead.
-func New(conn *dbus.Conn, path dbus.ObjectPath, props map[string]map[string]*Prop) *Properties {
+func New(conn *dbus.Conn, path dbus.ObjectPath, props Map) *Properties {
 	p, err := Export(conn, path, props)
 	if err != nil {
 		return nil
@@ -196,15 +198,33 @@ func New(conn *dbus.Conn, path dbus.ObjectPath, props map[string]map[string]*Pro
 // second-level key is the name of the property. The returned structure will be
 // exported as org.freedesktop.DBus.Properties on path.
 func Export(
-	conn *dbus.Conn, path dbus.ObjectPath, props map[string]map[string]*Prop,
+	conn *dbus.Conn, path dbus.ObjectPath, props Map,
 ) (*Properties, error) {
-	p := &Properties{m: props, conn: conn, path: path}
+	p := &Properties{m: copyProps(props), conn: conn, path: path}
 	if err := conn.Export(p, path, "org.freedesktop.DBus.Properties"); err != nil {
 		return nil, err
 	}
 	return p, nil
 }
 
+// Map is a helper type for supplying the configuration of properties to be handled.
+type Map = map[string]map[string]*Prop
+
+func copyProps(in Map) Map {
+	out := make(Map, len(in))
+	for intf, props := range in {
+		out[intf] = make(map[string]*Prop)
+		for name, prop := range props {
+			out[intf][name] = new(Prop)
+			*out[intf][name] = *prop
+			val := reflect.New(reflect.TypeOf(prop.Value))
+			val.Elem().Set(reflect.ValueOf(prop.Value))
+			out[intf][name].Value = val.Interface()
+		}
+	}
+	return out
+}
+
 // Get implements org.freedesktop.DBus.Properties.Get.
 func (p *Properties) Get(iface, property string) (dbus.Variant, *dbus.Error) {
 	p.mut.RLock()
@@ -217,7 +237,7 @@ func (p *Properties) Get(iface, property string) (dbus.Variant, *dbus.Error) {
 	if !ok {
 		return dbus.Variant{}, ErrPropNotFound
 	}
-	return dbus.MakeVariant(prop.Value), nil
+	return dbus.MakeVariant(reflect.ValueOf(prop.Value).Elem().Interface()), nil
 }
 
 // GetAll implements org.freedesktop.DBus.Properties.GetAll.
@@ -230,7 +250,7 @@ func (p *Properties) GetAll(iface string) (map[string]dbus.Variant, *dbus.Error)
 	}
 	rm := make(map[string]dbus.Variant, len(m))
 	for k, v := range m {
-		rm[k] = dbus.MakeVariant(v.Value)
+		rm[k] = dbus.MakeVariant(reflect.ValueOf(v.Value).Elem().Interface())
 	}
 	return rm, nil
 }
@@ -240,7 +260,7 @@ func (p *Properties) GetAll(iface string) (map[string]dbus.Variant, *dbus.Error)
 func (p *Properties) GetMust(iface, property string) interface{} {
 	p.mut.RLock()
 	defer p.mut.RUnlock()
-	return p.m[iface][property].Value
+	return reflect.ValueOf(p.m[iface][property].Value).Elem().Interface()
 }
 
 // Introspection returns the introspection data that represents the properties
@@ -260,9 +280,6 @@ func (p *Properties) Introspection(iface string) []introspect.Property {
 // must already be locked.
 func (p *Properties) set(iface, property string, v interface{}) error {
 	prop := p.m[iface][property]
-	if reflect.ValueOf(prop.Value).Kind() != reflect.Ptr {
-		prop.Value = reflect.New(reflect.TypeOf(prop.Value)).Interface()
-	}
 	err := dbus.Store([]interface{}{v}, prop.Value)
 	if err != nil {
 		return err
@@ -324,9 +341,8 @@ func (p *Properties) Set(iface, property string, newv dbus.Variant) *dbus.Error
 func (p *Properties) SetMust(iface, property string, v interface{}) {
 	p.mut.Lock()
 	defer p.mut.Unlock() // unlock in case of panic
-	prop := p.m[iface][property]
-	prop.Value = v
-	if err := p.emitChange(iface, property); err != nil {
+	err := p.set(iface, property, v)
+	if err != nil {
 		panic(err)
 	}
 }

prop/prop_test.go

@@ -100,3 +100,47 @@ func TestValidateStructsAsProp(t *testing.T) {
 	comparePropValue(obj, "FooStructPtr", *zooPtr, t)
 	comparePropValue(obj, "SliceOfFoos", zoos, t)
 }
+
+func TestInt32(t *testing.T) {
+	srv, err := dbus.SessionBus()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer srv.Close()
+
+	cli, err := dbus.SessionBus()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer cli.Close()
+
+	propsSpec := map[string]map[string]*Prop{
+		"org.guelfey.DBus.Test": {
+			"int32": {
+				int32(100),
+				true,
+				EmitTrue,
+				nil,
+			},
+		},
+	}
+	props := New(srv, "/org/guelfey/DBus/Test", propsSpec)
+
+	obj := cli.Object(srv.Names()[0], "/org/guelfey/DBus/Test")
+
+	comparePropValue(obj, "int32", int32(100), t)
+	r := props.GetMust("org.guelfey.DBus.Test", "int32")
+	if r != int32(100) {
+		t.Errorf("expected r to be int32(100), but was %#v", r)
+	}
+
+	if err := props.Set("org.guelfey.DBus.Test", "int32", dbus.MakeVariant(int32(101))); err != nil {
+		t.Fatalf("failed to set prop int32 to 101")
+	}
+
+	comparePropValue(obj, "int32", int32(101), t)
+	r = props.GetMust("org.guelfey.DBus.Test", "int32")
+	if r != int32(101) {
+		t.Errorf("expected r to be int32(101), but was %#v", r)
+	}
+}