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

[David Holmes]

On 23/06/2016 9:33 PM, serguei.spitsyn at oracle.com wrote:
> 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/>

As Stanislav said explicitly mentioning the empty string is not really 
necessary - but I don't see it as harmful.

> Does it looks Ok?

Yes - as good now as it was in the CCC discussion :)

Cheers,
David

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