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

[serguei.spitsyn at oracle.com]

On 6/29/16 01:45, Jesper Wilhelmsson wrote:
> 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,

Thank you for the comment.
In fact, I've just used a pattern that is already present in the JVMTI spec.
Objections to the existing pattern are minor, so it is better to be more 
conservative here.

Thanks,
Serguei



> /Jesper
>
>
>> Thanks,
>> David
>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>>> David
>>>>
>>>>
>>>>> Thanks,
>>>>> Serguei
>>>>>
>>>>>
>>>>>>
>>>>>> Dan