[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Request for review: 7064927: retransformClasses() does not pass in LocalVariableTable of a method
From:       serguei.spitsyn () oracle ! com (serguei ! spitsyn at oracle ! com)
Date:       2011-12-28 7:02:01
Message-ID: 4EFABEE9.8040508 () oracle ! com
[Download RAW message or body]

Looks good.
One comment though.

The attr_count should  be incremented under the same condition as the 
attribute is written:

  226   if (local_variable_table_length != 0) {
  227     write_local_variable_table_attribute(method, local_variable_table_length);
  228   }

It means, the line 176 should be after the line 177:

  174   if (method->has_localvariable_table()) {
  175     local_variable_table_length = method->localvariable_table_length();
  176     ++attr_count;
  177     if (local_variable_table_length != 0) {

Example is:

  144   if (const_method->has_linenumber_table()) {
  145     line_num_cnt = line_number_table_entries(method);
  146     if (line_num_cnt != 0) {
  147       ++attr_count;


The LVTT attribute is still missed though. :(


Thanks,
Serguei

On 12/21/11 12:35 PM, Thomas Wuerthinger wrote:
> Thanks for the review. I've corrected the comments and created a new 
> webrev at: http://cr.openjdk.java.net/~thomaswue/7064927/webrev.00/
> 
> - thomas
> 
> 
> On 20.12.2011 23:09, Paul Hohensee wrote:
> > Looks good, except that the LocalVariableTable attribute comments in
> > write_code_attribute() and write_local_variable_table_attribute() don't
> > match.  The comment in the latter says "LineNumberTable" instead
> > of "LocalVariableTable" and the descriptor is for the line number table
> > rather than the local variable table.  The comment in the former is 
> > missing
> > "local_variable_table[local_variable_table_length];"
> > 
> > Paul
> > 
> > On 12/20/11 4:26 PM, Thomas Wuerthinger wrote:
> > > Hi!
> > > 
> > > I'd like to resend my request for review and kindly ask for a review 
> > > for my patch that fixes bug number 7064927.
> > > The webrev is available at: 
> > > http://cr.openjdk.java.net/~coleenp/7064927/
> > > 
> > > Thanks, thomas
> > > 
> > > 
> > > On 14.11.2011 12:18, Thomas Wuerthinger wrote:
> > > > Hi!
> > > > 
> > > > An open webrev of the patch that fixes bug 7064927 is available at: 
> > > > http://cr.openjdk.java.net/~coleenp/7064927/
> > > > The bug is described at: 
> > > > http://bugs.sun.com/view_bug.do?bug_id=7064927
> > > > 
> > > > Thanks, thomas
> > > 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20111227/c6978be7/attachment.html \



[prev in list] [next in list] [prev in thread] [next in thread]