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

[Jesper Wilhelmsson]

29 juni 2016 kl. 04:30 skrev David Holmes <david.holmes at oracle.com>:
> 
>> 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.

I would prefer if it said "points to a NULL pointer", or NULL-pointer. I'm not sure what would be the more correct way to write it. Anyway, a NULL pointer is an entity :-)
/Jesper


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