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

[serguei.spitsyn at oracle.com]

On 6/23/16 04:27, David Holmes wrote:
> On 23/06/2016 9:08 PM, serguei.spitsyn at oracle.com wrote:
>> On 6/23/16 03:51, David Holmes wrote:
>>> On 23/06/2016 6:04 PM, serguei.spitsyn at oracle.com wrote:
>>>> On 6/23/16 00:51, Alan Bateman wrote:
>>>>>
>>>>>
>>>>> On 23/06/2016 00:20, serguei.spitsyn at 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.
>>>>> The primary use-case for this function is from within the CLFH
>>>>> callback so have it return the unnamed module when calling with 
>>>>> garage
>>>>> is probably okay, it just needs to be specified.
>>>>>
>>>>> Will you update the webrev to reflect where we've got to this in this
>>>>> discussion?
>>>>
>>>> I need to undo my fix for additional check.
>>>> My up-to-date understanding is that we have to explicitly specify two
>>>> points:
>>>>   - the function returns the unnamed module if the package does not
>>>> exist
>>>>   - the function does not check the package names for validness and
>>>> always returns the unnamed module for illegal package names
>>>>
>>>> It is better to specify these points explicitly to avoid possible
>>>> confusion.
>>>
>>> I don't agree - nothing else refers to invalid "names" in JVM TI. I
>>> don't see any need to call this out here. If the name supplied is not
>>> a legal package name then it will not match - end of story.
>>
>> David,
>>
>> It was the original approach that is in the CCC.
>> You were the one who got confused here, and it convinced me that this
>> needs to be more clear.
>> All these changes are because you started the discussion. :)
>> It was useful anyway.
>
> I never said anything about illegal package names.

True.
This is my (probably, wrong) attempt to make it more clear.

>
>> Are you against both clarifications or just the last one?
>> I'm Ok to return it back as it is in the CCC.
>> It will save time on the CCC approval.
>>
>> This is the last addition to the spec:
>>
>> + The unnamed module is returned if the specified package does not 
>> exist.
>
> The notion of "package does not exist" is ill-defined. This case is 
> already covered by the primary specification.
>
>> + The function does not check if the specified package name is illegal.
>
> This does not need to be stated as it is not stated anywhere else for 
> anything else that may have legal and illegal forms.

Good.
This is a relevant fragment from current version of CCC:

+      <description>
+        Return the <code>java.lang.reflect.Module</code> object for a 
module
+        defined to a class loader that contains a given package.
+        The module is returned via <code>module_ptr</code>.
+        <p/>
+        If a named module is defined to the class loader and it
+        contains the package then that named module is returned,
+        otherwise the unnamed module of the class loader is returned.
+        If the package name is the empty string then this function
+        always returns the unnamed module for the class loader.
+        <p/>

Does it looks Ok?

Thanks,
Serguei

>
> Thanks,
> David
>
>>
>>
>> Thanks,
>> Serguei
>>
>>
>>
>>
>>>
>>> Cheers,
>>> David
>>>
>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>>
>>>>
>>>>>
>>>>> -Alan
>>>>
>>