store: copy Nodes correctly in NodeExtern.Clone

store/node_extern.go

@@ -77,7 +77,7 @@ func (eNode *NodeExtern) Clone() *NodeExtern {
 	if eNode == nil {
 		return nil
 	}
-	en := &NodeExtern{
+	nn := &NodeExtern{
 		Key:           eNode.Key,
 		Dir:           eNode.Dir,
 		TTL:           eNode.TTL,
@@ -86,17 +86,19 @@ func (eNode *NodeExtern) Clone() *NodeExtern {
 	}
 	if eNode.Value != nil {
 		s := *eNode.Value
-		en.Value = &s
+		nn.Value = &s
 	}
 	if eNode.Expiration != nil {
 		t := *eNode.Expiration
-		en.Expiration = &t
+		nn.Expiration = &t
 	}
-	eNode.Nodes = make(NodeExterns, len(en.Nodes))
-	for i, n := range en.Nodes {
-		eNode.Nodes[i] = n.Clone()
+	if eNode.Nodes != nil {
+		nn.Nodes = make(NodeExterns, len(eNode.Nodes))
+		for i, n := range eNode.Nodes {
+			nn.Nodes[i] = n.Clone()
+		}
 	}
-	return en
+	return nn
 }
 
 type NodeExterns []*NodeExtern

store/node_extern_test.go

@@ -1,8 +1,10 @@
 package store
 
 import (
+	"reflect"
 	"testing"
 	"time"
+	"unsafe"
 )
 import "github.com/coreos/etcd/Godeps/_workspace/src/github.com/stretchr/testify/assert"
 
@@ -19,10 +21,13 @@ func TestNodeExternClone(t *testing.T) {
 		mi  uint64 = 321
 	)
 	var (
-		val  = "some_data"
-		valp = &val
-		exp  = time.Unix(12345, 67890)
-		expp = &exp
+		val    = "some_data"
+		valp   = &val
+		exp    = time.Unix(12345, 67890)
+		expp   = &exp
+		child  = NodeExtern{}
+		childp = &child
+		childs = []*NodeExtern{childp}
 	)
 
 	eNode = &NodeExtern{
@@ -32,6 +37,7 @@ func TestNodeExternClone(t *testing.T) {
 		ModifiedIndex: mi,
 		Value:         valp,
 		Expiration:    expp,
+		Nodes:         childs,
 	}
 
 	gNode := eNode.Clone()
@@ -43,6 +49,8 @@ func TestNodeExternClone(t *testing.T) {
 	// values should be the same
 	assert.Equal(t, *gNode.Value, val)
 	assert.Equal(t, *gNode.Expiration, exp)
+	assert.Equal(t, len(gNode.Nodes), len(childs))
+	assert.Equal(t, *gNode.Nodes[0], child)
 	// but pointers should differ
 	if gNode.Value == eNode.Value {
 		t.Fatalf("expected value pointers to differ, but got same!")
@@ -50,6 +58,9 @@ func TestNodeExternClone(t *testing.T) {
 	if gNode.Expiration == eNode.Expiration {
 		t.Fatalf("expected expiration pointers to differ, but got same!")
 	}
+	if sameSlice(gNode.Nodes, eNode.Nodes) {
+		t.Fatalf("expected nodes pointers to differ, but got same!")
+	}
 	// Original should be the same
 	assert.Equal(t, eNode.Key, key)
 	assert.Equal(t, eNode.TTL, ttl)
@@ -57,15 +68,26 @@ func TestNodeExternClone(t *testing.T) {
 	assert.Equal(t, eNode.ModifiedIndex, mi)
 	assert.Equal(t, eNode.Value, valp)
 	assert.Equal(t, eNode.Expiration, expp)
+	if !sameSlice(eNode.Nodes, childs) {
+		t.Fatalf("expected nodes pointer to same, but got different!")
+	}
 	// Change the clone and ensure the original is not affected
 	gNode.Key = "/baz"
 	gNode.TTL = 0
+	gNode.Nodes[0].Key = "uno"
 	assert.Equal(t, eNode.Key, key)
 	assert.Equal(t, eNode.TTL, ttl)
 	assert.Equal(t, eNode.CreatedIndex, ci)
 	assert.Equal(t, eNode.ModifiedIndex, mi)
+	assert.Equal(t, *eNode.Nodes[0], child)
 	// Change the original and ensure the clone is not affected
 	eNode.Key = "/wuf"
 	assert.Equal(t, eNode.Key, "/wuf")
 	assert.Equal(t, gNode.Key, "/baz")
 }
+
+func sameSlice(a, b []*NodeExtern) bool {
+	ah := (*reflect.SliceHeader)(unsafe.Pointer(&a))
+	bh := (*reflect.SliceHeader)(unsafe.Pointer(&b))
+	return *ah == *bh
+}