From 268959f4be869edd64588f248a50c7951e4456e5 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 11 Jun 2020 09:49:47 -0400 Subject: [PATCH] make UpEdges and DownEdges return a copy The public functions for the graph UpEdges and DownEdges is returning the internal Set from the graph, meaning that callers could inadvertently corrupt the graph structure by editing the returned Sets. Make UpEdges and DownEdges return a copy of the set, while retaining the efficient no-copy behavior for internal callers. --- dag/dag.go | 20 ++++++++++---------- dag/graph.go | 29 ++++++++++++++++++++++------- dag/set.go | 9 +++++++++ dag/tarjan.go | 2 +- 4 files changed, 42 insertions(+), 18 deletions(-) diff --git a/dag/dag.go b/dag/dag.go index 8ca4e910e..f16a459f6 100644 --- a/dag/dag.go +++ b/dag/dag.go @@ -36,7 +36,7 @@ func (g *AcyclicGraph) Ancestors(v Vertex) (Set, error) { return nil } - if err := g.DepthFirstWalk(g.DownEdges(v), memoFunc); err != nil { + if err := g.DepthFirstWalk(g.downEdgesNoCopy(v), memoFunc); err != nil { return nil, err } @@ -52,7 +52,7 @@ func (g *AcyclicGraph) Descendents(v Vertex) (Set, error) { return nil } - if err := g.ReverseDepthFirstWalk(g.UpEdges(v), memoFunc); err != nil { + if err := g.ReverseDepthFirstWalk(g.upEdgesNoCopy(v), memoFunc); err != nil { return nil, err } @@ -65,7 +65,7 @@ func (g *AcyclicGraph) Descendents(v Vertex) (Set, error) { func (g *AcyclicGraph) Root() (Vertex, error) { roots := make([]Vertex, 0, 1) for _, v := range g.Vertices() { - if g.UpEdges(v).Len() == 0 { + if g.upEdgesNoCopy(v).Len() == 0 { roots = append(roots, v) } } @@ -101,10 +101,10 @@ func (g *AcyclicGraph) TransitiveReduction() { // // For each v-prime reachable from v, remove the edge (u, v-prime). for _, u := range g.Vertices() { - uTargets := g.DownEdges(u) + uTargets := g.downEdgesNoCopy(u) - g.DepthFirstWalk(g.DownEdges(u), func(v Vertex, d int) error { - shared := uTargets.Intersection(g.DownEdges(v)) + g.DepthFirstWalk(g.downEdgesNoCopy(u), func(v Vertex, d int) error { + shared := uTargets.Intersection(g.downEdgesNoCopy(v)) for _, vPrime := range shared { g.RemoveEdge(BasicEdge(u, vPrime)) } @@ -208,7 +208,7 @@ func (g *AcyclicGraph) DepthFirstWalk(start Set, f DepthWalkFunc) error { return err } - for _, v := range g.DownEdges(current.Vertex) { + for _, v := range g.downEdgesNoCopy(current.Vertex) { frontier = append(frontier, &vertexAtDepth{ Vertex: v, Depth: current.Depth + 1, @@ -248,7 +248,7 @@ func (g *AcyclicGraph) SortedDepthFirstWalk(start []Vertex, f DepthWalkFunc) err } // Visit targets of this in a consistent order. - targets := AsVertexList(g.DownEdges(current.Vertex)) + targets := AsVertexList(g.downEdgesNoCopy(current.Vertex)) sort.Sort(byVertexName(targets)) for _, t := range targets { @@ -285,7 +285,7 @@ func (g *AcyclicGraph) ReverseDepthFirstWalk(start Set, f DepthWalkFunc) error { } seen[current.Vertex] = struct{}{} - for _, t := range g.UpEdges(current.Vertex) { + for _, t := range g.upEdgesNoCopy(current.Vertex) { frontier = append(frontier, &vertexAtDepth{ Vertex: t, Depth: current.Depth + 1, @@ -325,7 +325,7 @@ func (g *AcyclicGraph) SortedReverseDepthFirstWalk(start []Vertex, f DepthWalkFu seen[current.Vertex] = struct{}{} // Add next set of targets in a consistent order. - targets := AsVertexList(g.UpEdges(current.Vertex)) + targets := AsVertexList(g.upEdgesNoCopy(current.Vertex)) sort.Sort(byVertexName(targets)) for _, t := range targets { frontier = append(frontier, &vertexAtDepth{ diff --git a/dag/graph.go b/dag/graph.go index 4ce0dbccb..1d0544354 100644 --- a/dag/graph.go +++ b/dag/graph.go @@ -111,10 +111,10 @@ func (g *Graph) Remove(v Vertex) Vertex { g.vertices.Delete(v) // Delete the edges to non-existent things - for _, target := range g.DownEdges(v) { + for _, target := range g.downEdgesNoCopy(v) { g.RemoveEdge(BasicEdge(v, target)) } - for _, source := range g.UpEdges(v) { + for _, source := range g.upEdgesNoCopy(v) { g.RemoveEdge(BasicEdge(source, v)) } @@ -137,10 +137,10 @@ func (g *Graph) Replace(original, replacement Vertex) bool { // Add our new vertex, then copy all the edges g.Add(replacement) - for _, target := range g.DownEdges(original) { + for _, target := range g.downEdgesNoCopy(original) { g.Connect(BasicEdge(replacement, target)) } - for _, source := range g.UpEdges(original) { + for _, source := range g.upEdgesNoCopy(original) { g.Connect(BasicEdge(source, replacement)) } @@ -166,14 +166,29 @@ func (g *Graph) RemoveEdge(edge Edge) { } } -// DownEdges returns the outward edges from the source Vertex v. +// UpEdges returns the vertices connected to the outward edges from the source +// Vertex v. +func (g *Graph) UpEdges(v Vertex) Set { + return g.upEdgesNoCopy(v).Copy() +} + +// DownEdges returns the vertices connected from the inward edges to Vertex v. func (g *Graph) DownEdges(v Vertex) Set { + return g.downEdgesNoCopy(v).Copy() +} + +// downEdgesNoCopy returns the outward edges from the source Vertex v as a Set. +// This Set is the same as used internally bu the Graph to prevent a copy, and +// must not be modified by the caller. +func (g *Graph) downEdgesNoCopy(v Vertex) Set { g.init() return g.downEdges[hashcode(v)] } -// UpEdges returns the inward edges to the destination Vertex v. -func (g *Graph) UpEdges(v Vertex) Set { +// upEdgesNoCopy returns the inward edges to the destination Vertex v as a Set. +// This Set is the same as used internally bu the Graph to prevent a copy, and +// must not be modified by the caller. +func (g *Graph) upEdgesNoCopy(v Vertex) Set { g.init() return g.upEdges[hashcode(v)] } diff --git a/dag/set.go b/dag/set.go index f3fd704ba..c5c1af120 100644 --- a/dag/set.go +++ b/dag/set.go @@ -103,3 +103,12 @@ func (s Set) List() []interface{} { return r } + +// Copy returns a shallow copy of the set. +func (s Set) Copy() Set { + c := make(Set) + for k, v := range s { + c[k] = v + } + return c +} diff --git a/dag/tarjan.go b/dag/tarjan.go index 330abd589..fb4d4a773 100644 --- a/dag/tarjan.go +++ b/dag/tarjan.go @@ -24,7 +24,7 @@ func stronglyConnected(acct *sccAcct, g *Graph, v Vertex) int { index := acct.visit(v) minIdx := index - for _, raw := range g.DownEdges(v) { + for _, raw := range g.downEdgesNoCopy(v) { target := raw.(Vertex) targetIdx := acct.VertexIndex[target]