Code review request 7190219 CharBuffer position changed after BufferOverflowException in put() (original) (raw)

Jonathan Lu luchsh at linux.vnet.ibm.com
Mon Aug 13 08:26:52 UTC 2012

Hello Alan,

Thanks for reviewing, I've updated the webrev, could you please take a look?

http://cr.openjdk.java.net/~luchsh/7190219_2/

On 08/09/2012 09:28 PM, Alan Bateman wrote:

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 at 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.

In the updated webrev, I'm using relGet() to perform the content checking after put(). And I also updated all the generated Basic.java files using genBasic.sh.

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.

Thanks & regards Jonathan

More information about the core-libs-dev mailing list