Code review request for 6992121: StringBuilder.ensureCapacity(int minCap) throws OutOfMemoryError

[David Holmes]

Mandy Chung said the following on 10/19/10 15:05:
>  On 10/18/10 9:24 PM, David Holmes wrote:
>> Hi Mandy,
>>
>> Given you can't actually test ArrayList I would drop it altogether and 
>> move the test to java/util/vector/TestEnsureCapacity
> 
> But it tests ArrayList.ensureCapacity(Integer.MIN_VALUE) which should 
> not throw any exception (the regression throws OutOfMemoryError).  

Oh ok. I hadn't picked up on that part. Just leave it then.

David

I can
> move it to java/util/vector/TestEnsureCapacity.
> 
> Thanks
> Mandy
> 
>>
>> David
>>
>> Mandy Chung said the following on 10/19/10 14:07:
>>>  On 10/18/10 4:54 PM, David Holmes wrote:
>>>> Hi Mandy,
>>>>
>>>> The functional changes seem ok. However the ArrayList test is not 
>>>> right because it checks the size not the capacity. The same with the 
>>>> Vector test, though at least with Vector you can check capacity().
>>>>
>>>
>>> I missed the Vector.capacity() method.  Thanks.   I updated the test.
>>>    http://cr.openjdk.java.net/~mchung/6992121/webrev.01/
>>>> I find it odd that ArrayList has a very well defined notion of 
>>>> capacity yet no method to query that capacity.
>>>>
>>>
>>> True.  I don't have the history but wonder if this is intentional.
>>>
>>> Mandy
>>>
>>>> David
>>>>
>>>> Mandy Chung said the following on 10/19/10 08:41:
>>>>>  Please review the fix for:
>>>>>
>>>>> 6992121: StringBuilder.ensureCapacity(int minCap) throws 
>>>>> OutOfMemoryError with minCap=Integer.MIN_VALUE
>>>>>
>>>>> Webrev at:
>>>>>     http://cr.openjdk.java.net/~mchung/6992121/webrev.00/
>>>>>
>>>>> This is a regression caused by the changes for 6933217 (Huge arrays 
>>>>> handled poorly in core libraries).   The ensureCapacity() method in 
>>>>> the StringBuffer, StringBuilder, ArrayList, and Vector classes are 
>>>>> a public API that accepts negative numbers.   The following 
>>>>> overflow-conscious code:
>>>>>
>>>>>   if (minimumCapacity - value.length>  0)
>>>>>
>>>>> considers negative minimumCapacity as overflow case where 
>>>>> OutOfMemoryError will be thrown.  So the implementation of the 
>>>>> public ensureCapacity() methods need to check if the input argument 
>>>>> is positive before doing the above overflow-conscious check.
>>>>>
>>>>> Thanks
>>>>> Mandy
>>>
>