RFR: 8161448: 4 JNI exception pending defect groups in DiagnosticCommandImpl.c

[Harsha Wardhana B]

Hello All,

Please find below the revised webrev below.

http://cr.openjdk.java.net/~hb/8161448/webrev.01/

Regards

Harsha


On Wednesday 24 August 2016 11:48 AM, Harsha Wardhana B wrote:
> Hi David,
>
>
> On Tuesday 23 August 2016 12:41 PM, David Holmes wrote:
>> Hi Harsha,
>>
>> On 22/08/2016 6:30 PM, Harsha Wardhana B wrote:
>>> Hello All,
>>>
>>> Please review the below parfait fixes for DiagnosticCommandImpl.c
>>>
>>> Issue : https://bugs.openjdk.java.net/browse/JDK-8161448
>>>
>>> webrev : http://cr.openjdk.java.net/~hb/8161448/webrev.00/
>>
>> Looks functionally correct, but I wouldn't complain if you wanted to 
>> use a macro to reduce the duplication and verbosity. Even the:
>>
>>  109     if (obj == NULL) {
>>  110       free(dcmd_arg_info_array);
>>  111       return NULL;
>>
>> can be replaced by an exception-check as that is the only time 
>> JNU_NewObjectByName can return NULL.
> I am not sure if using macro here is good practice since it will be 
> specific to the function and it will encapsulate the local variable 
> within it. Also, it will reduce code readability. Hence I would like 
> to leave it as is.
>>
>> I also noticed an issue that was flagged in some other JNI code lately:
>>
>>  213       if (obj == NULL) {
>>  214           free(dcmd_info_array);
>>  215           JNU_ThrowOutOfMemoryError(env, 0);
>>  216           return NULL;
>>
>> If obj == NULL then there is already a pending exception and we 
>> should not be throwing OOME.
>>
> Sure. This needs to be fixed.
>> Thanks,
>> David
>>
>>> Regards
>>>
>>> Harsha
>>>
> Harsha