RFR(XS): 4965252: JvmtiExport::post_raw_field_modification jni ref handling is odd

[David Holmes]

Hi Rickard,

On 15/05/2013 7:37 PM, Rickard Bäckman wrote:
> can I please have this change reviewed?
>
> My interpretation is that this isn't really a bug, since the parameter sig_type is never set to [.

Right - it looks like a bug visually but turns out to be dead code. FYI 
the  "sig_type == '['" was added to the if under 4639363 - no idea why. 
I don't see [ ever being used for sigtype.

> The suggested change is to remove the check for [ in the if and add an assert. I also created a boolean
> to track handle creation to simplify cleanup.

Not sure the boolean was worth the effort - the existing check for L 
suffices. :)

Even the assert seems overly conservative given the single caller :)

Still trying to decide what was meant in the bug report about the lack 
of an "enclosing handle block" ??

Good to go if it hasn't already :)

Cheers,
David


>
> The caller of this method is InterpreterRuntime::post_field_modification
> which sets the sig_type to:
>
>    switch(cp_entry->flag_state()) {
>      case btos: sig_type = 'Z'; break;
>      case ctos: sig_type = 'C'; break;
>      case stos: sig_type = 'S'; break;
>      case itos: sig_type = 'I'; break;
>      case ftos: sig_type = 'F'; break;
>      case atos: sig_type = 'L'; break;
>      case ltos: sig_type = 'J'; break;
>      case dtos: sig_type = 'D'; break;
>      default:  ShouldNotReachHere(); return;
>    }
>
> Testing done: nsk.jvmti.testlist with fastdebug build.
>
> Webrev: http://cr.openjdk.java.net/~rbackman/4965252/
>
> Thanks
> /R
>
>
>