RFR(XS): 8163973: VM Anonymous classes should not call Class File Load Hooks

[Rachel Protacio]

Hi David and Coleen,

Thank you for the reviews. I've updated the change as requested: 
http://cr.openjdk.java.net/~rprotacio/8163973.01/

Rachel

On 8/16/2016 10:04 PM, Coleen Phillimore wrote:
>
>
> On 8/15/16 11:36 PM, David Holmes wrote:
>> Hi Rachel,
>>
>> On 16/08/2016 6:48 AM, Rachel Protacio wrote:
>>> Hello,
>>>
>>> Please review this change, which makes sure class file load hooks are
>>> not called for VM anonymous classes. See
>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-January/038353.html 
>>>
>>> for justification.
>>>
>>> Passes JPRT and RBT.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8163973
>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8163973.00/
>>
>> This:
>>
>>  112   // VM Anonymous classes - defined via 
>> unsafe.DefineAnonymousClass - should not
>>  113   // call back to a CFLH
>>  114   if (host_klass == NULL) {
>>  115     stream = prologue(stream,
>>
>> suggests that "prologue" can only do CFLH related things. If that is 
>> true then it would be much clearer in my opinion if prologue were 
>> renamed to something more explicit - like check_class_file_load_hook 
>> ? Otherwise, the host_klass should be passed in to prologue and the 
>> anonymous class check internalized there.
>
> I agree with David here.  This was sort of bothering me about your 
> change when we talked about it before.   If the prologue did more than 
> call the CFLH then you'd have to pass host_klass down, since we know 
> that's all it does, the name of the function should be changed.   
> David's name looks good to me.
>>
>> Also I don't think you need to explain where VM anonymous classes 
>> come from, it suffices to simply say "Skip class file load hook 
>> processing for VM anonymous classes"; or if the prologue is renamed 
>> then simply "Skip this processing for VM anonymous classes". :)
>
> Thanks,
> Coleen
>>
>> Thanks,
>> David
>>
>>> Thank you!
>>> Rachel
>