Harsha, I'm for macro.
You can define a macro right before a place where you uses it and undef it when you don't need it anymore. -Dmitry On 2016-08-25 11:19, Harsha Wardhana B wrote: > 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 > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
