harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Ellison <t.p.elli...@gmail.com>
Subject Re: [classlib] Request for golden ticket to update String class (was: Re: [classlib] Exception throwing compatibility)
Date Sun, 14 May 2006 04:00:19 GMT
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

-- 

Tim Ellison (t.p.ellison@gmail.com)
IBM Java technology centre, UK.

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