Dmitry,
Please, see inlined.
On 8/21/12 11:47 AM, Dmitry Samersoff wrote:
Serguei,
jvmtiClassFileReconstituter.cpp:
178: attr_count was 1 but become 0 for the case
has_localvariable_table() == true && local_variable_table_length == 0
is it intentional?
Yes.
You can see the same pattern for all attributes (for instance,
has_stackmap_table - L160).
202: local_variable_type_table_length
is always 0 if local_variable_table_length == 0
so I think if should be nested.
It does not matter.
216: It might be better to place both attr_size changes together.
i.e.
if (local_variable_table_length != 0) {
++attr_count;
LocalVariableTableElement *elem = method->localvariable_table_start();
for (int idx = 0; idx < local_variable_table_length; idx++) {
...
}
// Big comment block
attr_size += 2 + 4 + 2 + local_variable_table_length ...
if (local_variable_type_table_length != 0) {
++attr_count;
attr_size += 2 + 4 + 2 + local_variable_type_table_length ...
}
}
I'm trying to follow the existing style.
But thank you for sharing your thoughts on this!
Thanks,
Serguei
-Dmitry
On 2012-08-21 22:13, [email protected] wrote:
Hello,
Please, review the fix for:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7191786
Open webrev:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2012/7191786-JVMTI-LVTT/
Summary:
The following CR was recently fixed:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7064927
But the same issue exists for the LocalVariableTypeTable attribute.
The fix was tested with the modified test:
test/java/lang/instrument/VerifyLocalVariableTableOnRetransformTest.sh
The modification is that a local variable with generic signature is added
to the class DummyClassWithLVT.java:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2012/7191786-JLI-Test-For-JVMTI-LVTT/
The test patch will be integrated into the jdk8/tl after the HotSpot fix
is promoted.
Thanks,
Serguei
