godbus/dbus

Error when shrinking a slice property

Closed this issue · 2 comments

I am writing a dbus server which exposes an ObjectPath slice as a property. Adding a new element to the slice and calling prop.Set works as expected, but trying to remove an element from the slice fails with: dbus.Store: type mismatch: slices are different lengths need: 0 have: 1 due to this check:

dbus/dbus.go

Lines 281 to 286 in 8e438d3

if dest.Len() != src.Len() {
return fmt.Errorf(
"dbus.Store: type mismatch: "+
"slices are different lengths "+
"need: %d have: %d",
src.Len(), dest.Len())
.

Is there a reason why this check exists? The behavior seems to work correctly with the following change:

diff --git i/dbus.go w/dbus.go
index e9d014f..32ab49f 100644
--- i/dbus.go
+++ w/dbus.go
@@ -275,16 +275,9 @@ func storeSliceIntoInterface(dest, src reflect.Value) error {
 }
 
 func storeSliceIntoSlice(dest, src reflect.Value) error {
-	if dest.IsNil() || dest.Len() < src.Len() {
+	if dest.IsNil() || dest.Len() != src.Len() {
 		dest.Set(reflect.MakeSlice(dest.Type(), src.Len(), src.Cap()))
 	}
-	if dest.Len() != src.Len() {
-		return fmt.Errorf(
-			"dbus.Store: type mismatch: "+
-				"slices are different lengths "+
-				"need: %d have: %d",
-			src.Len(), dest.Len())
-	}
 	for i := 0; i < src.Len(); i++ {
 		err := store(dest.Index(i), getVariantValue(src.Index(i)))
 		if err != nil {

I can open a PR to fix this if that is a valid change, otherwise some guidance on how to do this properly would be appreciated.

Thanks!

Nice catch! Indeed, in this case we don't even need to call reflect.MakeSlice again, but could just fill dest and reslice it with reflect.Slice to avoid some allocation. Feel free to open a PR for that.

Thanks for the feedback :) I opened a PR to fix this; not sure if I interpreted your comment correctly so please let me know if I need to change something.