Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

Stanislav and David, Thank you for this interesting discussion! It seems, there are good points from both sides. Alan suggested to replace the GetModuleByPackageName with the GetNamedModule. The GetNamedModule should return NULL in all cases where the unnamed module was returned by the

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 24/06/2016 10:10 PM, stanislav lukyanov wrote: On 24.06.2016 2:07, serguei.spit...@oracle.com wrote: On 6/23/16 15:14, David Holmes wrote: On 23/06/2016 11:02 PM, stanislav lukyanov wrote: I think it should be specified that unnamed module will be returned even if there cannot ever be a

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 24.06.2016 2:07, serguei.spit...@oracle.com wrote: On 6/23/16 15:14, David Holmes wrote: On 23/06/2016 11:02 PM, stanislav lukyanov wrote: I think it should be specified that unnamed module will be returned even if there cannot ever be a package with that name. It is not a complicated

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 6/23/16 15:14, David Holmes wrote: On 23/06/2016 11:02 PM, stanislav lukyanov wrote: There were different points in the discussion I didn't have a chance to comment in time, so I'll just summarize everything here not to break the thread flow with answers to older messages. Thank you for

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 23/06/2016 11:02 PM, stanislav lukyanov wrote: There were different points in the discussion I didn't have a chance to comment in time, so I'll just summarize everything here not to break the thread flow with answers to older messages. First of all, I'm perfectly fine with either approach

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

There were different points in the discussion I didn't have a chance to comment in time, so I'll just summarize everything here not to break the thread flow with answers to older messages. First of all, I'm perfectly fine with either approach (adding name validation or not) as long as the

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 6/23/16 04:40, David Holmes wrote: On 23/06/2016 9:33 PM, serguei.spit...@oracle.com wrote: On 6/23/16 04:27, David Holmes wrote: On 23/06/2016 9:08 PM, serguei.spit...@oracle.com wrote: On 6/23/16 03:51, David Holmes wrote: On 23/06/2016 6:04 PM, serguei.spit...@oracle.com wrote: On

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 23/06/2016 9:33 PM, serguei.spit...@oracle.com wrote: On 6/23/16 04:27, David Holmes wrote: On 23/06/2016 9:08 PM, serguei.spit...@oracle.com wrote: On 6/23/16 03:51, David Holmes wrote: On 23/06/2016 6:04 PM, serguei.spit...@oracle.com wrote: On 6/23/16 00:51, Alan Bateman wrote: On

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 6/23/16 04:27, David Holmes wrote: On 23/06/2016 9:08 PM, serguei.spit...@oracle.com wrote: On 6/23/16 03:51, David Holmes wrote: On 23/06/2016 6:04 PM, serguei.spit...@oracle.com wrote: On 6/23/16 00:51, Alan Bateman wrote: On 23/06/2016 00:20, serguei.spit...@oracle.com wrote: : I

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 23/06/2016 9:08 PM, serguei.spit...@oracle.com wrote: On 6/23/16 03:51, David Holmes wrote: On 23/06/2016 6:04 PM, serguei.spit...@oracle.com wrote: On 6/23/16 00:51, Alan Bateman wrote: On 23/06/2016 00:20, serguei.spit...@oracle.com wrote: : I agree with it. Thank you for pointing to

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 6/23/16 03:51, David Holmes wrote: On 23/06/2016 6:04 PM, serguei.spit...@oracle.com wrote: On 6/23/16 00:51, Alan Bateman wrote: On 23/06/2016 00:20, serguei.spit...@oracle.com wrote: : I agree with it. Thank you for pointing to this JVMTI example. I did not find in the JNI where the

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

Hi Andrew, You should not try to convince me that updating webrev in place is a generally bad idea. I have the same opinion. :) Just did not want to generate another round for a pretty small change. Sorry. Thanks, Serguei On 6/23/16 01:52, Andrew Dinn wrote: On 23/06/16 09:41,

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 23/06/16 09:41, serguei.spit...@oracle.com wrote: > Updated the webrev in place: > > Hotspot: > http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.2 Just as an aside, I think it's actually a bad idea to update in place. Much better to just add new webrevs.

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

Updated the webrev in place: Hotspot: http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.2 Please, let me know about your opinion. Do I have to update the CCC as well? Thanks, Serguei On 6/23/16 01:02, Alan Bateman wrote: On 23/06/2016 08:57, David Holmes

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 6/23/16 00:51, Alan Bateman wrote: On 23/06/2016 00:20, serguei.spit...@oracle.com wrote: : I agree with it. Thank you for pointing to this JVMTI example. I did not find in the JNI where the names are checked to be legal. We are going to open a can of worms with this kind of check as

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 23/06/2016 08:57, David Holmes wrote: It already is specified :) If the name is garbage then it is, by definition, not a package that is associated with any module hence the else clause kicks in and the unnamed module is returned. Sure but there are are other changes the wording in this

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 23/06/2016 5:51 PM, Alan Bateman wrote: On 23/06/2016 00:20, serguei.spit...@oracle.com wrote: : I agree with it. Thank you for pointing to this JVMTI example. I did not find in the JNI where the names are checked to be legal. We are going to open a can of worms with this kind of check as

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 23/06/2016 00:20, serguei.spit...@oracle.com wrote: : I agree with it. Thank you for pointing to this JVMTI example. I did not find in the JNI where the names are checked to be legal. We are going to open a can of worms with this kind of check as there can be many corner cases to cover.

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 6/22/16 16:40, David Holmes wrote: On 23/06/2016 9:20 AM, serguei.spit...@oracle.com wrote: On 6/22/16 15:31, David Holmes wrote: On 23/06/2016 5:01 AM, Alan Bateman wrote: On 22/06/2016 19:36, serguei.spit...@oracle.com wrote: Alan, please, let me know if you support the Stanislav's

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

Hi Serguei, What is the expected behavior when passed string is not a valid package name (e.g. contains illegal characters)? I think I'd expect JVMTI_ERROR_ILLEGAL_ARGUMENT to be returned in that case. Now it looks like the method will return unnamed module not only for an empty string but

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 23/06/2016 9:20 AM, serguei.spit...@oracle.com wrote: On 6/22/16 15:31, David Holmes wrote: On 23/06/2016 5:01 AM, Alan Bateman wrote: On 22/06/2016 19:36, serguei.spit...@oracle.com wrote: Alan, please, let me know if you support the Stanislav's suggestion. I can go ahead and implement

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 6/22/16 15:31, David Holmes wrote: On 23/06/2016 5:01 AM, Alan Bateman wrote: On 22/06/2016 19:36, serguei.spit...@oracle.com wrote: Alan, please, let me know if you support the Stanislav's suggestion. I can go ahead and implement returning the JVMTI_ERROR_ILLEGAL_ARGUMENT for invalid

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 23/06/2016 5:01 AM, Alan Bateman wrote: On 22/06/2016 19:36, serguei.spit...@oracle.com wrote: Alan, please, let me know if you support the Stanislav's suggestion. I can go ahead and implement returning the JVMTI_ERROR_ILLEGAL_ARGUMENT for invalid package names. No objection from me. It

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 6/22/16 13:54, Jiangli Zhou wrote: Hi Serguei, On Jun 22, 2016, at 1:17 PM, serguei.spit...@oracle.com wrote: Hi Jiangli, Fixed both places - nice catch! Would you like I list you as a reviewer? No worry, if you already committed/pushed your changes.

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

Hi Serguei, > On Jun 22, 2016, at 1:17 PM, serguei.spit...@oracle.com wrote: > > Hi Jiangli, > > Fixed both places - nice catch! > Would you like I list you as a reviewer? No worry, if you already committed/pushed your changes. Thanks, Jiangli > > Thanks, > Serguei > > > On 6/22/16 10:29,

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

Hi Jiangli, Fixed both places - nice catch! Would you like I list you as a reviewer? Thanks, Serguei On 6/22/16 10:29, Jiangli Zhou wrote: Hi Serguei, The comment in the following assert is outdated. There is no get_module_from_pkg. Should it be changed to get_module_by_package_name()?

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 6/22/16 12:01, Alan Bateman wrote: On 22/06/2016 19:36, serguei.spit...@oracle.com wrote: Alan, please, let me know if you support the Stanislav's suggestion. I can go ahead and implement returning the JVMTI_ERROR_ILLEGAL_ARGUMENT for invalid package names. No objection from me. Ok,

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 22/06/2016 19:36, serguei.spit...@oracle.com wrote: Alan, please, let me know if you support the Stanislav's suggestion. I can go ahead and implement returning the JVMTI_ERROR_ILLEGAL_ARGUMENT for invalid package names. No objection from me. It would be good to first check the existing

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 6/22/16 11:11, serguei.spit...@oracle.com wrote: On 6/22/16 10:45, Alan Bateman wrote: On 22/06/2016 18:09, stanislav lukyanov wrote: Hi Serguei, What is the expected behavior when passed string is not a valid package name (e.g. contains illegal characters)? I think I'd expect

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

Hi Stanislav, Thank you for looking at this update and the comments! On 6/22/16 10:09, stanislav lukyanov wrote: Hi Serguei, What is the expected behavior when passed string is not a valid package name (e.g. contains illegal characters)? I think I'd expect JVMTI_ERROR_ILLEGAL_ARGUMENT to be

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 6/22/16 10:45, Alan Bateman wrote: On 22/06/2016 18:09, stanislav lukyanov wrote: Hi Serguei, What is the expected behavior when passed string is not a valid package name (e.g. contains illegal characters)? I think I'd expect JVMTI_ERROR_ILLEGAL_ARGUMENT to be returned in that case.

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

On 22/06/2016 18:09, stanislav lukyanov wrote: Hi Serguei, What is the expected behavior when passed string is not a valid package name (e.g. contains illegal characters)? I think I'd expect JVMTI_ERROR_ILLEGAL_ARGUMENT to be returned in that case. Now it looks like the method will return

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

Hi Serguei, The comment in the following assert is outdated. There is no get_module_from_pkg. Should it be changed to get_module_by_package_name()? 804 assert(ModuleEntryTable::javabase_defined(), 805 "Attempt to call get_module_from_pkg before java.base is defined"); It’s not

Re: Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

I've updated the libGetModuleByPkgTest.c in place in the webrev #2 with minor improvements. Sorry, if it spoiled your work. Thanks, Serguei On 6/22/16 04:07, serguei.spit...@oracle.com wrote: Here are new hotspot webrev where I've fixed the comments from Alan and Christian. Hotspot:

Jigsaw Enhancement RFR round #2: 8159145 Add JVMTI function GetModuleByPackageName

Here are new hotspot webrev where I've fixed the comments from Alan and Christian. Hotspot: http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8159145-jigsaw-jvmti-pkg.2 The Jdk webrev is the same: