harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gregory Shimansky" <gshiman...@gmail.com>
Subject Re: [DRLVM][JNI]GetByteArrayRegion differs from RI (was Re: Exceptions found while running servlet...)
Date Tue, 15 Aug 2006 12:21:54 GMT
2006/8/15, Jimmy, Jing Lv <firepure@gmail.com>:
>
>      IMHO our Compatibility guidelines, if the spec is not clear, we
> should follow RI. So no matter what happens to the spec(unless it
> describe the detail condition of exception-thrown), it is still OK to
> follow RI here, am I right?


Ok you didn't convince me which spec if more correct but to close this
discussion I agree to create a patch to JIRA 1156 to allow start index equal
to array length in case the len argument is 0.

In classlib discussions I've seen we agreed to document places where we
chose to follow RI instead of public spec. Is there any place where we will
document these conditions for VMs?

> The whole exception condition looks like this
> >
> >     jsize length = GetArrayLength(env, array);
> >     jsize end = start + len;
> >     if(start < 0 || start >= length || end < 0 || end > length) {
> >         char msg[30];
> >         sprintf(msg, "%d..%d", start, end);
> >         ThrowNew_Quick(env, "java/lang/ArrayIndexOutOfBoundsException",
> msg);
> >         return;
> >     }
> >
> > and it is easy to change start >= length to start > length if you ask
> for it.
>
> IMHO there's something wrong here. What will it do if len<0 and length
> >start+len >0 ? Shall it be:
> if(start < 0 || start > length || len < 0 || end > length)?
>
> And the next line shall be like:
> if (0 == len){
>         return;
> }


No. There is assert(len >= 0) right before the code snippet which I pasted.
JNI does not allow users to pass incorrect arguments like null pointers and
bad indexes so it is legal to use assertions in its implementation [1]
(another link to JNI spec which doesn't convince anyone here any more).

So the code correction which I proposed seems to be the only needed one. I
don't think it is necessary to handle len == 0 specifically to avoid
additional checks for a rare (hopefully) case.

> I am still unsure if this is a place where spec should step away from the
> > spec be it imcomplete or not. Programmers who don't work for Sun read
> public
> > spec, don't they?
> >
> > [1] http://java.sun.com/docs/books/jni/html/jniTOC.html
> >
> Still we should follow our Compatibility guidelines, right(For our goal
> of Harmony)?  :)
>

These guidelines were so far dicussed for classlib only. While I agree with
most of them they mention only classlib implementation so far.

[1] http://java.sun.com/j2se/1.5.0/docs/guide/jni/spec/design.html#wp17593

-- 
Gregory Shimansky, Intel Middleware Products Division

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message