RFR: 8313452: Improve Classfile API attributes handling safety [v3]
Brian Goetz
briangoetz at openjdk.org
Thu Sep 14 16:37:45 UTC 2023
On Wed, 6 Sep 2023 15:03:24 GMT, Adam Sotona <asotona at 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 incrementally with one additional commit since the last revision:
>
> magic moved to Util::isAttributeAllowed and fixed possible NPE
Marked as reviewed by briangoetz (Reviewer).
src/java.base/share/classes/jdk/internal/classfile/AttributeMapper.java line 117:
> 115: * {@return attribute stability indicator}
> 116: */
> 117: AttributeStability attributeStability();
Perhaps the name `stability()` would be better here because all the methods here are about the attribute being described.
src/java.base/share/classes/jdk/internal/classfile/impl/AbstractDirectBuilder.java line 58:
> 56: attributes.withAttribute(a);
> 57: }
> 58: }
The "is allowed" check should always say yes for unbound attributes, and only apply the option for bound attributes. (You may already have done that, but I wanted to make sure this was captured in the review.)
src/java.base/share/classes/jdk/internal/classfile/impl/Util.java line 64:
> 62: ? ATTRIBUTE_STABILITY_COUNT - attr.attributeMapper().attributeStability().ordinal() > processingOption.ordinal()
> 63: : true;
> 64: }
Ignore earlier comment, looks like you've got this :)
-------------
PR Review: https://git.openjdk.org/jdk/pull/15101#pullrequestreview-1627325194
PR Review Comment: https://git.openjdk.org/jdk/pull/15101#discussion_r1326230242
PR Review Comment: https://git.openjdk.org/jdk/pull/15101#discussion_r1326233656
PR Review Comment: https://git.openjdk.org/jdk/pull/15101#discussion_r1326235687
More information about the core-libs-dev
mailing list