From 1858ad65c8e28eb6cc0b289480afc5b717597c1c Mon Sep 17 00:00:00 2001 From: James Tucker Date: Thu, 14 Sep 2023 10:52:37 -0700 Subject: [PATCH] cmd/cloner: do not allocate slices when the source is nil tailcfg.Node zero-value clone equality checks failed when I added a []*foo to the structure, as the zero value and it's clone contained a different slice header. Updates #9377 Updates #9408 Signed-off-by: James Tucker --- cmd/cloner/cloner.go | 2 ++ cmd/viewer/tests/tests_clone.go | 32 ++++++++++++++++++++------------ tailcfg/tailcfg_clone.go | 32 ++++++++++++++++++++------------ wgengine/filter/filter_clone.go | 16 ++++++++++------ wgengine/wgcfg/wgcfg_clone.go | 8 +++++--- 5 files changed, 57 insertions(+), 33 deletions(-) diff --git a/cmd/cloner/cloner.go b/cmd/cloner/cloner.go index 4e8e2119b..5a94fa97d 100644 --- a/cmd/cloner/cloner.go +++ b/cmd/cloner/cloner.go @@ -122,6 +122,7 @@ func gen(buf *bytes.Buffer, it *codegen.ImportTracker, typ *types.Named) { case *types.Slice: if codegen.ContainsPointers(ft.Elem()) { n := it.QualifiedName(ft.Elem()) + writef("if src.%s != nil {", fname) writef("dst.%s = make([]%s, len(src.%s))", fname, n, fname) writef("for i := range dst.%s {", fname) if ptr, isPtr := ft.Elem().(*types.Pointer); isPtr { @@ -137,6 +138,7 @@ func gen(buf *bytes.Buffer, it *codegen.ImportTracker, typ *types.Named) { writef("\tdst.%s[i] = *src.%s[i].Clone()", fname, fname) } writef("}") + writef("}") } else { writef("dst.%s = append(src.%s[:0:0], src.%s...)", fname, fname, fname) } diff --git a/cmd/viewer/tests/tests_clone.go b/cmd/viewer/tests/tests_clone.go index 4d82c03bd..2b41639fd 100644 --- a/cmd/viewer/tests/tests_clone.go +++ b/cmd/viewer/tests/tests_clone.go @@ -136,21 +136,29 @@ func (src *StructWithSlices) Clone() *StructWithSlices { dst := new(StructWithSlices) *dst = *src dst.Values = append(src.Values[:0:0], src.Values...) - dst.ValuePointers = make([]*StructWithoutPtrs, len(src.ValuePointers)) - for i := range dst.ValuePointers { - dst.ValuePointers[i] = src.ValuePointers[i].Clone() + if src.ValuePointers != nil { + dst.ValuePointers = make([]*StructWithoutPtrs, len(src.ValuePointers)) + for i := range dst.ValuePointers { + dst.ValuePointers[i] = src.ValuePointers[i].Clone() + } } - dst.StructPointers = make([]*StructWithPtrs, len(src.StructPointers)) - for i := range dst.StructPointers { - dst.StructPointers[i] = src.StructPointers[i].Clone() + if src.StructPointers != nil { + dst.StructPointers = make([]*StructWithPtrs, len(src.StructPointers)) + for i := range dst.StructPointers { + dst.StructPointers[i] = src.StructPointers[i].Clone() + } } - dst.Structs = make([]StructWithPtrs, len(src.Structs)) - for i := range dst.Structs { - dst.Structs[i] = *src.Structs[i].Clone() + if src.Structs != nil { + dst.Structs = make([]StructWithPtrs, len(src.Structs)) + for i := range dst.Structs { + dst.Structs[i] = *src.Structs[i].Clone() + } } - dst.Ints = make([]*int, len(src.Ints)) - for i := range dst.Ints { - dst.Ints[i] = ptr.To(*src.Ints[i]) + if src.Ints != nil { + dst.Ints = make([]*int, len(src.Ints)) + for i := range dst.Ints { + dst.Ints[i] = ptr.To(*src.Ints[i]) + } } dst.Slice = append(src.Slice[:0:0], src.Slice...) dst.Prefixes = append(src.Prefixes[:0:0], src.Prefixes...) diff --git a/tailcfg/tailcfg_clone.go b/tailcfg/tailcfg_clone.go index 9d3ca5221..6fb2c81b5 100644 --- a/tailcfg/tailcfg_clone.go +++ b/tailcfg/tailcfg_clone.go @@ -219,9 +219,11 @@ func (src *DNSConfig) Clone() *DNSConfig { } dst := new(DNSConfig) *dst = *src - dst.Resolvers = make([]*dnstype.Resolver, len(src.Resolvers)) - for i := range dst.Resolvers { - dst.Resolvers[i] = src.Resolvers[i].Clone() + if src.Resolvers != nil { + dst.Resolvers = make([]*dnstype.Resolver, len(src.Resolvers)) + for i := range dst.Resolvers { + dst.Resolvers[i] = src.Resolvers[i].Clone() + } } if dst.Routes != nil { dst.Routes = map[string][]*dnstype.Resolver{} @@ -229,9 +231,11 @@ func (src *DNSConfig) Clone() *DNSConfig { dst.Routes[k] = append([]*dnstype.Resolver{}, src.Routes[k]...) } } - dst.FallbackResolvers = make([]*dnstype.Resolver, len(src.FallbackResolvers)) - for i := range dst.FallbackResolvers { - dst.FallbackResolvers[i] = src.FallbackResolvers[i].Clone() + if src.FallbackResolvers != nil { + dst.FallbackResolvers = make([]*dnstype.Resolver, len(src.FallbackResolvers)) + for i := range dst.FallbackResolvers { + dst.FallbackResolvers[i] = src.FallbackResolvers[i].Clone() + } } dst.Domains = append(src.Domains[:0:0], src.Domains...) dst.Nameservers = append(src.Nameservers[:0:0], src.Nameservers...) @@ -365,9 +369,11 @@ func (src *DERPRegion) Clone() *DERPRegion { } dst := new(DERPRegion) *dst = *src - dst.Nodes = make([]*DERPNode, len(src.Nodes)) - for i := range dst.Nodes { - dst.Nodes[i] = src.Nodes[i].Clone() + if src.Nodes != nil { + dst.Nodes = make([]*DERPNode, len(src.Nodes)) + for i := range dst.Nodes { + dst.Nodes[i] = src.Nodes[i].Clone() + } } return dst } @@ -444,9 +450,11 @@ func (src *SSHRule) Clone() *SSHRule { if dst.RuleExpires != nil { dst.RuleExpires = ptr.To(*src.RuleExpires) } - dst.Principals = make([]*SSHPrincipal, len(src.Principals)) - for i := range dst.Principals { - dst.Principals[i] = src.Principals[i].Clone() + if src.Principals != nil { + dst.Principals = make([]*SSHPrincipal, len(src.Principals)) + for i := range dst.Principals { + dst.Principals[i] = src.Principals[i].Clone() + } } dst.SSHUsers = maps.Clone(src.SSHUsers) dst.Action = src.Action.Clone() diff --git a/wgengine/filter/filter_clone.go b/wgengine/filter/filter_clone.go index d945afd3d..e34ae7685 100644 --- a/wgengine/filter/filter_clone.go +++ b/wgengine/filter/filter_clone.go @@ -24,9 +24,11 @@ func (src *Match) Clone() *Match { dst.IPProto = append(src.IPProto[:0:0], src.IPProto...) dst.Srcs = append(src.Srcs[:0:0], src.Srcs...) dst.Dsts = append(src.Dsts[:0:0], src.Dsts...) - dst.Caps = make([]CapMatch, len(src.Caps)) - for i := range dst.Caps { - dst.Caps[i] = *src.Caps[i].Clone() + if src.Caps != nil { + dst.Caps = make([]CapMatch, len(src.Caps)) + for i := range dst.Caps { + dst.Caps[i] = *src.Caps[i].Clone() + } } return dst } @@ -47,9 +49,11 @@ func (src *CapMatch) Clone() *CapMatch { } dst := new(CapMatch) *dst = *src - dst.Values = make([]json.RawMessage, len(src.Values)) - for i := range dst.Values { - dst.Values[i] = append(src.Values[i][:0:0], src.Values[i]...) + if src.Values != nil { + dst.Values = make([]json.RawMessage, len(src.Values)) + for i := range dst.Values { + dst.Values[i] = append(src.Values[i][:0:0], src.Values[i]...) + } } return dst } diff --git a/wgengine/wgcfg/wgcfg_clone.go b/wgengine/wgcfg/wgcfg_clone.go index f96277cb3..939f08e3f 100644 --- a/wgengine/wgcfg/wgcfg_clone.go +++ b/wgengine/wgcfg/wgcfg_clone.go @@ -24,9 +24,11 @@ func (src *Config) Clone() *Config { *dst = *src dst.Addresses = append(src.Addresses[:0:0], src.Addresses...) dst.DNS = append(src.DNS[:0:0], src.DNS...) - dst.Peers = make([]Peer, len(src.Peers)) - for i := range dst.Peers { - dst.Peers[i] = *src.Peers[i].Clone() + if src.Peers != nil { + dst.Peers = make([]Peer, len(src.Peers)) + for i := range dst.Peers { + dst.Peers[i] = *src.Peers[i].Clone() + } } return dst }