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 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.

Thanks,
David

Regards

Harsha

Reply via email to