container/list: fix Remove bug and use pointer to self as identifier

Remove wasn't nil'ing the *Element.id. This property was exploited
by MoveToFront and MoveToBack internally, so I renamed the existing
Remove to "remove", and created an exported wrapper "Remove" that does
the right thing for the user's sake.

Also, saved an allocation by using *List as the id rather than *byte.

Fixes #1224.

R=rsc, dsymonds
CC=golang-dev
https://golang.org/cl/2685042

diff --git a/src/pkg/container/list/list.go b/src/pkg/container/list/list.go
index 55831e8e61ef9737c5abe75006d0cab9651add44..47ceae170c5089ad818d85b4eb6a525107ba0fcb 100644 (file)
--- a/src/pkg/container/list/list.go
+++ b/src/pkg/container/list/list.go
@@ -11,8 +11,8 @@ type Element struct {
        // The front of the list has prev = nil, and the back has next = nil.
        next, prev *Element
 
-       // A unique ID for the list to which this element belongs.
-       id *byte
+       // Thie list to which this element belongs.
+       list *List
 
        // The contents of this list element.
        Value interface{}
@@ -29,7 +29,6 @@ func (e *Element) Prev() *Element { return e.prev }
 type List struct {
        front, back *Element
        len         int
-       id          *byte
 }
 
 // Init initializes or clears a List.
@@ -37,7 +36,6 @@ func (l *List) Init() *List {
        l.front = nil
        l.back = nil
        l.len = 0
-       l.id = new(byte)
        return l
 }
 
@@ -52,7 +50,15 @@ func (l *List) Back() *Element { return l.back }
 
 // Remove removes the element from the list.
 func (l *List) Remove(e *Element) {
-       if e.id != l.id {
+       l.remove(e)
+       e.list = nil // do what remove does not
+}
+
+// remove the element from the list, but do not clear the Element's list field.
+// This is so that other List methods may use remove when relocating Elements
+// without needing to restore the list field.
+func (l *List) remove(e *Element) {
+       if e.list != l {
                return
        }
        if e.prev == nil {
@@ -121,59 +127,59 @@ func (l *List) insertBack(e *Element) {
 
 // PushFront inserts the value at the front of the list and returns a new Element containing the value.
 func (l *List) PushFront(value interface{}) *Element {
-       if l.id == nil {
+       if l == nil {
                l.Init()
        }
-       e := &Element{nil, nil, l.id, value}
+       e := &Element{nil, nil, l, value}
        l.insertFront(e)
        return e
 }
 
 // PushBack inserts the value at the back of the list and returns a new Element containing the value.
 func (l *List) PushBack(value interface{}) *Element {
-       if l.id == nil {
+       if l == nil {
                l.Init()
        }
-       e := &Element{nil, nil, l.id, value}
+       e := &Element{nil, nil, l, value}
        l.insertBack(e)
        return e
 }
 
 // InsertBefore inserts the value immediately before mark and returns a new Element containing the value.
 func (l *List) InsertBefore(value interface{}, mark *Element) *Element {
-       if mark.id != l.id {
+       if mark.list != l {
                return nil
        }
-       e := &Element{nil, nil, l.id, value}
+       e := &Element{nil, nil, l, value}
        l.insertBefore(e, mark)
        return e
 }
 
 // InsertAfter inserts the value immediately after mark and returns a new Element containing the value.
 func (l *List) InsertAfter(value interface{}, mark *Element) *Element {
-       if mark.id != l.id {
+       if mark.list != l {
                return nil
        }
-       e := &Element{nil, nil, l.id, value}
+       e := &Element{nil, nil, l, value}
        l.insertAfter(e, mark)
        return e
 }
 
 // MoveToFront moves the element to the front of the list.
 func (l *List) MoveToFront(e *Element) {
-       if e.id != l.id || l.front == e {
+       if e.list != l || l.front == e {
                return
        }
-       l.Remove(e)
+       l.remove(e)
        l.insertFront(e)
 }
 
 // MoveToBack moves the element to the back of the list.
 func (l *List) MoveToBack(e *Element) {
-       if e.id != l.id || l.back == e {
+       if e.list != l || l.back == e {
                return
        }
-       l.Remove(e)
+       l.remove(e)
        l.insertBack(e)
 }
 

diff --git a/src/pkg/container/list/list_test.go b/src/pkg/container/list/list_test.go
index 4538a0dcfd1698024f0e9bacf500e47c1d836b1f..1d44ff84e4a477b40358e1488e35cdb74b7f61b1 100644 (file)
--- a/src/pkg/container/list/list_test.go
+++ b/src/pkg/container/list/list_test.go
@@ -23,8 +23,7 @@ func checkListPointers(t *testing.T, l *List, es []*Element) {
                t.Errorf("l.back = %v, want %v", l.back, last)
        }
 
-       for i := 0; i < len(es); i++ {
-               e := es[i]
+       for i, e := range es {
                var e_prev, e_next *Element = nil, nil
                if i > 0 {
                        e_prev = es[i-1]
@@ -194,3 +193,17 @@ func TestExtending(t *testing.T) {
        l1.PushFrontList(l3)
        checkList(t, l1, []interface{}{1, 2, 3})
 }
+
+func TestRemove(t *testing.T) {
+       l := New()
+       e1 := l.PushBack(1)
+       e2 := l.PushBack(2)
+       checkListPointers(t, l, []*Element{e1, e2})
+       e := l.Front()
+       l.Remove(e)
+       checkListPointers(t, l, []*Element{e2})
+       checkListLen(t, l, 1)
+       l.Remove(e)
+       checkListPointers(t, l, []*Element{e2})
+       checkListLen(t, l, 1)
+}