On 09/08/2012 13:16, Jonathan Lu wrote:
Hi folks,
Here's a patch for bug 7190219, could you please help to have a look?
http://cr.openjdk.java.net/~luchsh/7190219/
According to the specification, in the method
java.nio.CharBuffer.put(String src, int start, int end).
If there are more chars to be copied from the string than remain in
this buffer, that is, if end - start > remaining(), then no chars are
transferred and a|BufferOverflowException|||
<cid:part1.03040006.09080809@linux.vnet.ibm.com> is thrown.
But actually the test case from that webrev proves that the buffer was
modified even after BufferOverflowException, so I suggest to add
additional check after checkBounds() call, in the same way as
java.nio.CharBuffer.put(char[] src, int offset, int length).
You're right, it's missing a check to make sure that there is remaining
space and the proposed change looks right to me. Just to keep things
locally consistent you can remove the braces around the throw new
BufferOverflowException.
I think the test could be improved. In particular it could duplicate the
buffer and then check that it is equals to the duplicate after the put
fails (in addition to checking that that the position hasn't changed).
That way you really check that the buffer content haven't been modified.
Do you mind seeing if you can add this to the existing unit test for the
buffer classes rather than adding a new test? You'll see the existing
tests in Basic.java and Basic-X.java.template.
I see you've submitted an incident via bugs.sun.com for this, I've moved
to the right place as:
7190219: (bf) CharBuffer.put(String,int,int) modifies position even if
BufferOverflowException thrown
-Alan.
