On Mon, 13 May 2024 17:15:15 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org> 
wrote:

>> Adam Sotona has updated the pull request incrementally with two additional 
>> commits since the last revision:
>> 
>>  - fixed error thrown by VerifierImpl
>>  - applied suggested changes
>
> src/java.base/share/classes/jdk/internal/classfile/impl/verifier/ParserVerifier.java
>  line 205:
> 
>> 203:     private void verifyAttribute(AttributedElement ae, Attribute<?> a, 
>> List<VerifyError> errors) {
>> 204:         int payLoad = ((BoundAttribute)a).payloadLen();
>> 205:         if (payLoad != switch (a) {
> 
> Please break this up - e.g. with an intermediate `foundPayload` variable - 
> having a multi-line switch nested as an if condition looks very jarring!

Is this method only supposed to check the attribute size? It would be nice 
perhaps to enhance this to enforce more structural constraints - I added a 
couple of comments in that direction, but there's many more (e.g. for instance 
make sure that any entry that morally points to a class/method is of the right 
kind)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/16809#discussion_r1598810642

Reply via email to