I'm committing this one, that fixes a couple of "woops!" I did in the last patch, as well as some other methods that were already broken (read: not my fault :)
Now it should pass all the public domain tck166 tests, except for subList. Thanks, Mario 2007-11-24 Mario Torre <[EMAIL PROTECTED]> * java/util/concurrent/CopyOnWriteArrayList.java: (addAll): fix implementation, now add elements in the correct position. (addAllAbsent): fixed typos (woops!). (remove(int)): fixed range in arraycopy that was causing for incorrect values to be inserted in the list. Refactored to give variables better names. (remove(Object)): refactored to give better names to variable. (listIterator): fix to set the starting index. (listIterator.previous): fix to decrement element position before returning the previous element in the iterator. -- Lima Software - http://www.limasoftware.net/ GNU Classpath Developer - http://www.classpath.org/ Fedora Ambassador - http://fedoraproject.org/wiki/MarioTorre Jabber: [EMAIL PROTECTED] pgp key: http://subkeys.pgp.net/ PGP Key ID: 80F240CF Fingerprint: BA39 9666 94EC 8B73 27FA FC7C 4086 63E3 80F2 40CF Please, support open standards: http://opendocumentfellowship.org/petition/ http://www.nosoftwarepatents.com/
### Eclipse Workspace Patch 1.0 #P classpath Index: java/util/concurrent/CopyOnWriteArrayList.java =================================================================== RCS file: /sources/classpath/classpath/java/util/concurrent/CopyOnWriteArrayList.java,v retrieving revision 1.4 diff -u -r1.4 CopyOnWriteArrayList.java --- java/util/concurrent/CopyOnWriteArrayList.java 23 Nov 2007 21:51:27 -0000 1.4 +++ java/util/concurrent/CopyOnWriteArrayList.java 24 Nov 2007 22:16:53 -0000 @@ -41,7 +41,9 @@ import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.io.Serializable; + import java.lang.reflect.Array; + import java.util.AbstractList; import java.util.Collection; import java.util.Iterator; @@ -395,15 +397,23 @@ */ public synchronized E remove(int index) { - E[] data = this.data; - E[] newData = (E[]) new Object[data.length - 1]; + if (index < 0 || index >= this.size()) + throw new IndexOutOfBoundsException("index = " + index); + + E[] snapshot = this.data; + E[] newData = (E[]) new Object[snapshot.length - 1]; + + E result = snapshot[index]; + if (index > 0) - System.arraycopy(data, 0, newData, 0, index - 1); - System.arraycopy(data, index + 1, newData, index, - data.length - index - 1); - E r = data[index]; + System.arraycopy(snapshot, 0, newData, 0, index); + + System.arraycopy(snapshot, index + 1, newData, index, + snapshot.length - index - 1); + this.data = newData; - return r; + + return result; } /** @@ -417,32 +427,32 @@ */ public synchronized boolean remove(Object element) { - E[] data = this.data; - E[] newData = (E[]) new Object[data.length - 1]; + E[] snapshot = this.data; + E[] newData = (E[]) new Object[snapshot.length - 1]; // search the element to remove while filling the backup array // this way we can run this method in O(n) int elementIndex = -1; - for (int i = 0; i < this.data.length; i++) + for (int i = 0; i < snapshot.length; i++) { - if (equals(element, this.data[i])) + if (equals(element, snapshot[i])) { elementIndex = i; break; } if (i < newData.length) - newData[i] = this.data[i]; + newData[i] = snapshot[i]; } if (elementIndex < 0) return false; - - System.arraycopy(this.data, elementIndex + 1, newData, elementIndex, - this.data.length - elementIndex - 1); + + System.arraycopy(snapshot, elementIndex + 1, newData, elementIndex, + snapshot.length - elementIndex - 1); this.data = newData; - return false; + return true; } /** @@ -575,18 +585,33 @@ */ public synchronized boolean addAll(int index, Collection< ? extends E> c) { - E[] data = this.data; - Iterator<? extends E> itr = c.iterator(); + if (index < 0 || index > this.size()) + throw new IndexOutOfBoundsException("index = " + index); + int csize = c.size(); if (csize == 0) return false; - + + E[] data = this.data; + Iterator<? extends E> itr = c.iterator(); + E[] newData = (E[]) new Object[data.length + csize]; - System.arraycopy(data, 0, newData, 0, data.length); - int end = data.length; + + // avoid this call at all if we were asked to put the elements at the + // beginning of our storage + if (index != 0) + System.arraycopy(data, 0, newData, 0, index); + + int itemsLeft = index; + for (E value : c) - newData[end++] = value; + newData[index++] = value; + + // now copy the remaining elements + System.arraycopy(data, itemsLeft, newData, 0, data.length - itemsLeft); + this.data = newData; + return true; } @@ -624,7 +649,7 @@ size = 0; for (E val : c) { - if (this.contains(val)) + if (!this.contains(val)) storage[size++] = val; } @@ -635,7 +660,9 @@ E [] newData = (E[]) new Object[snapshot.length + size]; System.arraycopy(snapshot, 0, newData, 0, snapshot.length); - System.arraycopy(storage, 0, newData, snapshot.length + 1, storage.length); + System.arraycopy(storage, 0, newData, snapshot.length, size); + + this.data = newData; return size; } @@ -693,7 +720,7 @@ return new ListIterator<E>() { E [] iteratorData = CopyOnWriteArrayList.this.data; - int currentElement = 0; + int currentElement = index; public void add(E o) { @@ -730,7 +757,7 @@ if (hasPrevious() == false) throw new java.util.NoSuchElementException(); - return iteratorData[currentElement++]; + return iteratorData[--currentElement]; } public int previousIndex()