RFR: 8161448: 4 JNI exception pending defect groups in DiagnosticCommandImpl.c
Harsha Wardhana B
harsha.wardhana.b at oracle.com
Wed Aug 24 06:18:34 UTC 2016
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
More information about the serviceability-dev
mailing list