On Wed, 6 Sep 2023 12:28:05 GMT, Adam Sotona <asot...@openjdk.org> wrote:
>> This PR improved Classfile API attributes handling safety by introduction of >> attribute stability indicator >> and by extension of UnknownAttributesOption to more general >> AttributesProcessingOption. > > Adam Sotona has updated the pull request with a new target base due to a > merge or a rebase. The pull request now contains 23 commits: > > - fixed javadoc wording > - HAZMAT renamed to UNSTABLE > no filtering on read > filtering on write only for bound attributes > - Merge branch 'master' into JDK-8313452-attributes-safety > > # Conflicts: > # src/java.base/share/classes/jdk/internal/classfile/Classfile.java > - Merge branch 'JDK-8312491-snippets' into JDK-8313452-attributes-safety > - added notes to attributes > - fixing javadoc > - fixing javadoc > - fixing javadoc > - fixing javadoc > - removed extra lines > - ... and 13 more: https://git.openjdk.org/jdk/compare/744b3970...ce77e2ae src/java.base/share/classes/jdk/internal/classfile/impl/AbstractDirectBuilder.java line 59: > 57: if (a instanceof UnboundAttribute || > 58: 5 - a.attributeMapper().attributeStability().ordinal() > 59: > context.attributesProcessingOption().ordinal()) { Maybe extract the `5 - a.attributeMapper().attributeStability().ordinal() > context.attributesProcessingOption().ordinal()` check into a helper method, as the current expression is too noisy: import jdk.internal.classfile.AttributeMapper.AttributeStability; import jdk.internal.classfile.Classfile.AttributesProcessingOption; private static final int ATTRIBUTE_STABILITY_COUNT = AttributeStability.values().length; public static boolean isAttributeAllowed( final AttributeStability stability, final AttributesProcessingOption processingOption ) { return ATTRIBUTE_STABILITY_COUNT - stability.ordinal() > processingOption.ordinal(); } Also, the magic number should probably be replaced with a constant storing the value of `AttributeStability.values().length`. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15101#discussion_r1317326262
