[kaffe] Re: LinkedList.subList severely broken

Daniel Bonniot Daniel.Bonniot@inria.fr
Wed May 21 14:44:01 2003


This is a multi-part message in MIME format.
--------------060404050106000906080609
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

>
>
>> 		super(list, index);
>> 		elem = list.get(index);
>>
>>Which one is considered better?
>>    
>>
>
>simpler code is always better in my opinion. Easier to write, easier to
>understand, easier to change.
>
I quite agree!

I attach a corresponding patch, with a changelog entry.

The code 'list.get(index)' was obviously type incorrect, since it 
retrieved the contained value instead of the containing node. I had to 
use the findIndex method instead, whose visibility I changed from 
private to default. I don't think it matters much, since in LinkedList 
head and tail are already default, so direct manipulation from outside 
the class is already allowed.

I was quite surprised at the behaviour of the build system when some 
code in a library class doesn't compile. It seems that the class (and 
those that require it) are simply left out of rt.jar, but the build does 
not fail. Is there a good reason to do this? I would prefer to see the 
build fail.

Regarding Dalibor's patch queue status message: shouldn't my patch be 
applied in any case (merge reverted or not). It fixes a class 
(LinkedListIterator) which was not taken from Classpath. I don't know if 
this bug hapened not be to exercised before, or was just undiscovered. 
But the class is wrong anyway.

What will happen to the testcase? Has it been / will it be added to 
Kaffe testsuite (I have no idea what the format is)? Or better, to mauve 
(I could do it, if I though anybody card to include it)? Does one of 
Kaffe's developer has write access on Mauve (that would seem to make 
sense) ? Or is every VM mostly working on their own testsuite?

Cheers,

Daniel


--------------060404050106000906080609
Content-Type: text/plain;
 name="LinkedListIterator.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="LinkedListIterator.diff"

? kaffe/scripts/javah
? libraries/clib/awt/qt/.deps
? libraries/clib/awt/qt/Makefile
? libraries/extensions/sound/Makefile
? libraries/extensions/sound/alsa/.deps
? libraries/extensions/sound/common/.deps
? libraries/extensions/sound/common/.libs
? libraries/extensions/sound/common/common.lo
? libraries/extensions/sound/common/libtritonuscommon.la
? libraries/extensions/sound/esd/.deps
? replace/.deps
? replace/.libs
? replace/Makefile
? replace/getifaddrs.lo
? replace/libreplace.la
? test/internal/.deps
? test/internal/Makefile
Index: ChangeLog
===================================================================
RCS file: /cvs/kaffe/kaffe/ChangeLog,v
retrieving revision 1.1339
diff -u -r1.1339 ChangeLog
--- ChangeLog	19 May 2003 08:01:20 -0000	1.1339
+++ ChangeLog	21 May 2003 21:28:48 -0000
@@ -1,3 +1,9 @@
+2003-05-21  Daniel Bonniot  <bonniot@users.sourceforge.net>
+
+	* libraries/javalib/java/util/LinkedListIterator: 
+	(LinkedListIterator) make the iteration really take into account
+	the starting index.
+
 2003-05-19  Gwenole Beauchesne  <gbeauchesne@mandrakesoft.com>
 
         Add support for Linux/AMD64.
Index: libraries/javalib/java/util/LinkedList.java
===================================================================
RCS file: /cvs/kaffe/kaffe/libraries/javalib/java/util/LinkedList.java,v
retrieving revision 1.5
diff -u -r1.5 LinkedList.java
--- libraries/javalib/java/util/LinkedList.java	3 Dec 2001 03:42:07 -0000	1.5
+++ libraries/javalib/java/util/LinkedList.java	21 May 2003 21:29:18 -0000
@@ -290,7 +290,7 @@
 		return null;
 	}
 
-	private Elem findIndex(int index) {
+	Elem findIndex(int index) {
 		if (index < 0 || index >= length) {
 			throw new IndexOutOfBoundsException();
 		}
Index: libraries/javalib/java/util/LinkedListIterator.java
===================================================================
RCS file: /cvs/kaffe/kaffe/libraries/javalib/java/util/LinkedListIterator.java,v
retrieving revision 1.1
diff -u -r1.1 LinkedListIterator.java
--- libraries/javalib/java/util/LinkedListIterator.java	14 Jul 1999 00:53:24 -0000	1.1
+++ libraries/javalib/java/util/LinkedListIterator.java	21 May 2003 21:29:19 -0000
@@ -25,7 +25,7 @@
 
 	LinkedListIterator(LinkedList list, int index) {
 		super(list, index);
-		elem = list.head;
+		elem = list.findIndex(index);
 	}
 
 	public Object next() {

--------------060404050106000906080609--