Hi Joe,
I promise, I will not grumble about the formatting and
weird white spaces [grumble grumble]...
http://cr.openjdk.java.net/~joehw/jdk13/8224157/webrev/src/java.xml/share/classes/com/sun/org/apache/bcel/internal/classfile/ConstantUtf8.java.frames.html
Is the new import really needed here?
http://cr.openjdk.java.net/~joehw/jdk13/8224157/webrev/src/java.xml/share/classes/com/sun/org/apache/bcel/internal/classfile/JavaClass.java.frames.html
232 if (!dir.isDirectory()) {
I see use of SecuritySupport was dropped here and I think I agree,
as I don't see why isDirectory() would require it if mkdirs() doesn't.
http://cr.openjdk.java.net/~joehw/jdk13/8224157/webrev/src/java.xml/share/classes/com/sun/org/apache/bcel/internal/classfile/LineNumberTable.java.udiff.html
Are the changes in toString() actually correct?
Otherwise I believe the rest looks mostly OK.
best regards,
-- daniel
On 20/06/2019 23:45, Joe Wang wrote:
Please review an update to BCEL 6.3.1. This changeset will go into JDK13
if approved, 14 if not.
1. Format
The changeset looks big, the majority of the changes however were
only different in format (i.e. Const.java). Unlike previous updates, I'm
leaving the format as they are in the upstream source so that they won't
show up in future updates. The only change I made were those that had
extremely long lines.
2. Exclusions
Since the BCEL component is for xml transform only, several
classes, that were not in the JDK, were excluded. Among them,
JavaWrapper.java for example can be problematic as it may use an
user-specified classloader to load arbitrary classes. It and related
classes were therefore excluded.
3. Warnings
Warnings were the main reason for the changes made to the original
source. It has been done in the previous update. For this update
therefore, I only had to re-apply them after making copies of the
upstream source. Still, I updated the LastModified field to indicate a
modification to the original source.
4. Deprecated fields to private
Deprecated fields in the original source were changed to private
ones in previous update. The changes are inherited in this update.
Again, the LastModified fields are also updated.
5. Test
Since the update does not affect the usage of the BCEL component,
it is essential to pass all of the existing tests. I've run the tests
multiple times and noted that all of the XML functional and unit tests
passed, so were JCK XML tests. I've also done a comparison between
builds before and after applying the BCEL update, and found no change in
performance.
JBS: https://bugs.openjdk.java.net/browse/JDK-8224157
webrevs: http://cr.openjdk.java.net/~joehw/jdk13/8224157/webrev/
Thanks,
Joe
