Hi Brandon,
On 19/11/2011 11:21 AM, Brandon Passanisi wrote:
Hello core-libs-dev@openjdk.java.net. Please review the following patch
to fix Bug 4802647 (coll) NullPointerException not thrown by
AbstractCollection.retainAll/removeAll: The fix is quite small and I
have included a test program, all of which can be found within the
following webrev:
http://cr.openjdk.java.net/~mduigou/4802647/0/webrev/
The bug report is a little misleading. The bug only exists when the
current collection is empty - in which case we skip most of the method
body. Otherwise we will generate the NPE when we call
c.contains(iter.next()). The CR should be updated to note this.
So while your fix is correct, I hate to see good code penalized
(explicit null check) to allow bad code to throw exceptions as required.
But I don't see a way around it.
With regard to the test program - it can be simplified somewhat as
unexpected exceptions can just be allowed to propagate and failures to
throw can be detected inline:
public static void main(String[] args) {
// Ensure removeAll(null) throws NullPointerException
try {
(new NewAbstractCollection<>()).removeAll(null);
fail("NullPointerException not thrown for removeAll(null).");
} catch (NullPointerException expected) {
}
// Ensure retainAll(null) throws NullPointerException
try {
(new NewAbstractCollection<>()).retainAll(null);
fail("NullPointerException not thrown for retainAll(null).");
} catch (NullPointerException expected) {
}
}
Cheers,
David
Thanks.
