Jigsaw Enhancement RFR: 8159145 Add JVMTI function GetModuleByPackageName

[serguei.spitsyn at oracle.com]

Hi Alan,

Thank you for the review!
I'll implement all your suggestions.

Thanks,
Serguei


On 6/21/16 04:56, Alan Bateman wrote:
>
>
> On 21/06/2016 11:48, serguei.spitsyn at oracle.com wrote:
>>
>> Sorry.
>> My initial plan was to write an nsk.jvmti test and review it on a 
>> confidential mailing list.
>> It is why I put the webrevs on the non-public server.
>> Forgot to switch the server when the test was converted into the 
>> jtreg format.
>>
>> The public webrevs are:
>>
>> Hotspot:
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.1/ 
>>
>>
>> Jdk:
>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.jdk1/ 
>>
>>
>
> The spec generally looks good, just two small points:
>
> 1. "java.lang.ClassLoader" would be better than 
> "java/lang/ClassLoader" when describing the class_loader parameter.
>
> 2. For the package name then it might be clearer to say that the 
> package name is in internal form and then include the example of 
> "java/lang" in parenthesis.
>
> One comment on the test is that it looks like check_system_loader 
> doesn't test any packages that are defined to a module. 
> check_bootstrap_loader does have tests for named modules for the null 
> loader case. Just wondering if we need more here.
>
> You might have spotted this already but the header comment on the 
> tests needs to be the GPL header.
>
> -Alan