mirror of
https://github.com/golang/go
synced 2024-11-21 23:44:39 -07:00
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
This commit is contained in:
parent
ee065332dd
commit
01389b966e
@ -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)
|
||||
}
|
||||
|
||||
|
@ -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)
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user