harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Geir Magnusson Jr <g...@pobox.com>
Subject Re: [classlib] Request for golden ticket to update String class
Date Sun, 14 May 2006 15:56:34 GMT
<procedure_note>

I think that at this point, since Tim has nodded in agreement, and no 
one has objected, go forward.

I'm stating this so we don't turn this into a formal vote - you posted a 
note about something we agreed to take special care with, a reasonable 
amount of time has gone by, there is knowledgeable assent, and no 
complaints...

</procedure_note>

Tim Ellison wrote:
> I agree with your analysis.  +1 from me.
> 
> Regards,
> Tim
> 
> Stepan Mishura wrote:
>> On 5/12/06, *Jimmy, Jing Lv* wrote:
>>
>>     <SNIP>
>>     >> In this case, though replace StringIndexOutOfBoundsException with
>>     >> ArrayIndexOutOfBoundsException is surely better, it seems it is
>>     internal
>>     >> implementation what cause the problem. According to the code it use
>>     >> String.valueof(str), which writes:
>>     >> try {
>>     >>        System.arraycopy(data, start, value, 0, count);
>>     >> } catch (IndexOutOfBoundsException e) {
>>     >>        throw new StringIndexOutOfBoundsException();
>>     >> }
>>     >
>>     >
>>     > IMHO this try-catch block is redandunt - the method code already
>>     contains
>>     > checks to verify that all parameters are valid:
>>     > if (start >= 0 && 0 <= length && length <= data.length
- start) {
>>     >   ....
>>     > else
>>     >    throw new StringIndexOutOfBoundsException();
>>     >
>>
>>     I believe you are right, but there may be some reasons for the author
>>     to write such try{...}catch(){}, perhaps he do follow RI's exception in
>>     the class String.
>>
>>  
>> Hi,
>>  
>> I'd like to acquire golden ticket for updating String.java file. Please
>> review my request.
>>  
>> Description:
>> Constructor: public String(char[] data, int start, int length) contains
>> redundant try-catch block that should be removed:
>>     try {
>>         System.arraycopy(data, start, value, 0, count);
>>     } catch (IndexOutOfBoundsException e) {
>>         throw new StringIndexOutOfBoundsException();
>>     }
>>  
>> Basis:
>> According to the spec. method
>> System.arraycopy(Object src, int srcPos, Object dest, int destPos, int
>> length)
>>  
>> throws IndexOutOfBoundsException in the following cases:
>> 1) srcPos < 0
>> 2) destPos < 0
>> 3) length <0
>> 4) srcPos+length > src.length
>> 5) destPos+length > dest.length
>>  
>> In our case: destPos is passed as 0 value and dest array is allocated
>> as: value = new char[length]. So in 2 and 5 cases IOOBE cannot be thrown
>> by arraycopy method. For other cases (1,3,4) the constructor contains
>> explicit check and throws SIOOBE if passed parameters are invalid:
>> if (start >= 0 && 0 <= length && length <= data.length -
start) {
>>     ....
>> } else {
>>     throw new StringIndexOutOfBoundsException();
>>  
>> So System.arraycopy(...) method MUST not throw IOOBE in the the
>> constructor and try-catch block is redundant in this case.
>>  
>> Patch file is attached.
>>  
>> Thanks,
>> Stepan Mishura
>> Intel Middleware Products Division
>>
>> ------------------------------------------------------
>> Terms of use : http://incubator.apache.org/harmony/mailing.html
>> <http://incubator.apache.org/harmony/mailing.html>
>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
>> <mailto:harmony-dev-unsubscribe@incubator.apache.org>
>> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>> <mailto:harmony-dev-help@incubator.apache.org>
>>
>>
>> ------------------------------------------------------------------------
>>
>> Index: modules/luni/src/main/java/java/lang/String.java
>> ===================================================================
>> --- modules/luni/src/main/java/java/lang/String.java	(revision 405602)
>> +++ modules/luni/src/main/java/java/lang/String.java	(working copy)
>> @@ -434,11 +434,8 @@
>>  			offset = 0;
>>  			value = new char[length];
>>  			count = length;
>> -			try {
>> -				System.arraycopy(data, start, value, 0, count);
>> -			} catch (IndexOutOfBoundsException e) {
>> -				throw new StringIndexOutOfBoundsException();
>> -			}
>> +
>> +			System.arraycopy(data, start, value, 0, count);
>>  		} else
>>  			throw new StringIndexOutOfBoundsException();
>>  	}
>>
>>
>> ------------------------------------------------------------------------
>>
>> ---------------------------------------------------------------------
>> Terms of use : http://incubator.apache.org/harmony/mailing.html
>> To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
>> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> 

---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org


Mime
View raw message