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

[Jonathan Lu]

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