This is the mail archive of the java-patches@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] JDWP LocalVariables


Tom Tromey wrote:
"Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:

Kyle> This patch implements the functions to allow JDWP to get and set Kyle> local variables through JVMTI.

A few buglets in here but overall it is looking good.

Kyle> +void
Kyle> +checkJVMTIError (jvmtiEnv *env, jthread thread, jvmtiError jerr, jint slot,
Kyle> +                 jbyte sig)

A helper method like this, which isn't a class member, should be
'static'. There are several instances of this.
Didn't know that, but I'll make that change.
Kyle> +void
Kyle> +setLongJVMTI (jvmtiEnv *env, jthread thread, jint slot, jint depth, jbyte sig,
Kyle> +              jlong value)
Kyle> +{
Kyle> +  jvmtiError jerr = env->SetLocalInt (thread, depth, slot, value);

Cut-and-paste bug? Should be SetLocalLong.
This also affects the Float and Double cases.
Good spot, things like this have been giving us alot of problems.
Kyle> +  // If the method is non-static, we need to set the type for the "this" pointer.
Kyle> +  if ((method->accflags & java::lang::reflect::Modifier::STATIC) == 0)
Kyle> +    {
Kyle> +      frame_desc.locals_type[0] = 'o';
Kyle> +      type_ctr++;
Kyle> +    }

You might as well hoist this code, from later on, into this 'if':

Kyle> frame_desc.obj_ptr = locals[0].o;
Done.
Kyle> + for (int i = 0; signature[i] != ')' && i <= sig_len; i++)

Start with 'i = 1' to skip the '('.
Done.
Kyle> + else if (signature[i] == '[')
Kyle> [...]
Kyle> + Kyle> + // Check for an object array
Kyle> + if (signature[i] == 'L')
Kyle> + {
Kyle> + while (signature[i] != ';')
Kyle> + i++;
Kyle> + }
Kyle> + continue;
I think this won't do the right thing for a primitive array, like '[[I'.
In this case we need to skip the 'I' as well, so I think we need 'else ++i'
in there. What do you think?
This lookes wierd to me every time I look at it, but I think it's right
the way it is.  For example:

string[3] = "[[IF", i =0;

after while (string[i] == '[')
          i++;
i = 2, which points to 'I'.  then in the next loop iteration, since i
gets incremented i=3 and points to F.  I might, however, be crazy.

- Kyle


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]