You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Stepan Mishura <st...@gmail.com> on 2006/05/12 11:48:06 UTC
[classlib] Request for golden ticket to update String class (was: Re: [classlib] Exception throwing compatibility)
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
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org
Re: [classlib] Request for golden ticket to update String class
Posted by Geir Magnusson Jr <ge...@pobox.com>.
<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
>> <ma...@incubator.apache.org>
>> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
>> <ma...@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
Re: [classlib] Request for golden ticket to update String class
Posted by Geir Magnusson Jr <ge...@pobox.com>.
All opinions are welcome.
And in the future, fix your mailer so that it doesn't take the message
you are responding to as an attachment... see... nothing below your
statement of support...
geir
Ivan Volosyuk wrote:
> I like this solution. If my opinion is counted:
> +1 from me.
>
---------------------------------------------------------------------
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
Re: [classlib] Request for golden ticket to update String class (was: Re: [classlib] Exception throwing compatibility)
Posted by Ivan Volosyuk <iv...@gmail.com>.
I like this solution. If my opinion is counted:
+1 from me.
--
Ivan
Intel Middleware Products Division
2006/5/14, Tim Ellison <t....@gmail.com>:
> 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
> > <ma...@incubator.apache.org>
> > For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> > <ma...@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
>
>
---------------------------------------------------------------------
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
Re: [classlib] Request for golden ticket to update String class (was:
Re: [classlib] Exception throwing compatibility)
Posted by Tim Ellison <t....@gmail.com>.
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
> <ma...@incubator.apache.org>
> For additional commands, e-mail: harmony-dev-help@incubator.apache.org
> <ma...@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