Jigsaw Enhancement RFR round #3: 8159145 Add JVMTI function GetNamedModule

[David Holmes]

On 29/06/2016 12:09 PM, serguei.spitsyn at oracle.com wrote:
> On 6/28/16 18:44, David Holmes wrote:
>> On 29/06/2016 7:09 AM, serguei.spitsyn at oracle.com wrote:
>>> On 6/28/16 14:02, Daniel D. Daugherty wrote:
>>>> On 6/28/16 2:11 PM, serguei.spitsyn at oracle.com wrote:
>>>>> On 6/28/16 11:19, Daniel D. Daugherty wrote:
>>>>>
>>>>>>
>>>>>>         I'll have to check the upper layers of this API, but if an
>>>>>>         agent can pass a bad 'class_loader' parameter and get this
>>>>>>         assert() to fire, then that's not good. Hopefully a bad
>>>>>>         'class_loader' parameter is caught at a higher layer.
>>>>>
>>>>> Not sure, what do you mean.
>>>>> Do you mean the generated JVMTI upper layer or the
>>>>> JvmtiEnv::GetNamedModule?
>>>>> Probably, the generated code.
>>>>
>>>> I did mean the generated layer.
>>>
>>> Ok, thanks.
>>>
>>>>
>>>>
>>>>>
>>>>>>
>>>>>>         Update: Yes, passing a non-NULL jobject as the class_loader
>>>>>> parameter
>>>>>>             when the jobject does not refer to a "class loader" is
>>>>>> caught
>>>>>>             at the upper layer.
>>>>>
>>>>> The upper layer does not check that it is a class loader, just for
>>>>> non-NULL.
>>>>> I think, it is good to have an assert here to double-checks the
>>>>> pre-conditions as other caller can be added later.
>>>>> But I'm Ok to get rid of it if you suggest.
>>>>
>>>> Please keep the asserts. Basically I was mumbling to myself to
>>>> make sure that the asserts could not be reached by user code
>>>> and the "Update:" was to indicate that I did do.
>>>
>>> Ok, thanks.
>>>
>>>
>>>>
>>>>
>>>>>
>>>>>>
>>>>>> src/share/vm/prims/jvmti.xml
>>>>>>     L6550:         <param id="module_ptr">
>>>>>>     L6551: <outptr><jobject/></outptr>
>>>>>>     L6552:           <description>
>>>>>>     L6553:             On return, points to a
>>>>>> <code>java.lang.reflect.Module</code> object.
>>>>>>     L6554:           </description>
>>>>>>     L6555:         </param>
>>>>>>
>>>>>>         The above wording doesn't allow for module_ptr to be NULL
>>>>>> which
>>>>>>         is a mismatch with the description.
>>>>>
>>>>> I disagree (or maybe I got it incorrectly).
>>>>> Pointing to NULL and be NULL is different.
>>>>> It is not allowed for the module_ptr to be NULL but Ok to pint to
>>>>> NULL on return.
>>>>
>>>> I think the description needs to be:
>>>>
>>>>     On return, points to a <code>java.lang.reflect.Module</code> object
>>>>     or points to a <code>NULL</code>.
>>>
>>> Agreed, fixed.
>>
>> Disagree. You would say that a pointer is NULL, not that it points to
>> a NULL.
>
> Why are you disagree?
> The module_ptr is an out argument, it is not allowed to be NULL.
> But the returning value by this pointer can be NULL.

As per later email I see this terminology already exists so is fine, but 
I find it confusing to say "points to a NULL" - a NULL is not an entity. 
If "foo points to a NULL" that implies to me "foo == &NULL;" which is 
nonsense - foo == NULL, and if foo==NULL then foo points to nothing. But 
the "pointer to a pointer" aspect here does confuse things.

Thanks,
David

>
> Thanks,
> Serguei
>
>
>>
>> David
>>
>>
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>>>
>>>>
>>>> Dan
>>>
>