Jaroslav, Looks good for me!
-Dmitry On 2014-07-28 18:08, Jaroslav Bachorik wrote: > A few readability improvements suggested by Dmitry > > http://cr.openjdk.java.net/~jbachorik/8030115/webrev.04 > > -JB- > > > --- snipped for brevity --- > >>>>>> On 2014-07-24 15:07, Jaroslav Bachorik wrote: >>>>>>> Please, review the change for the fix of the following problem >>>>>>> >>>>>>> In jdk/src/share/native/sun/tracing/dtrace/JVM.c the following >>>>>>> code is >>>>>>> invoked in loop >>>>>>> >>>>>>> 198 for (i = 0; i < num_providers; ++i) { >>>>>>> 199 JVM_DTraceProvider* p = &(jvm_providers[i]); >>>>>>> 200 jobject provider = (*env)->GetObjectArrayElement( >>>>>>> 201 env, providers, i); >>>>>>> 202 readProviderData(env, provider, p); >>>>>>> 203 } >>>>>>> >>>>>>> The first problem is that GetGetObjectArrayElement() call on L200 >>>>>>> may >>>>>>> throw an exception which is not checked subsequently. On L202 the >>>>>>> call >>>>>>> to readProviderData() can also raise a Java exception without >>>>>>> appropriate after-check. When getting back to the beginning of the >>>>>>> loop >>>>>>> GetObjectArrayElement() may be called with a pending exception >>>>>>> which is >>>>>>> deemed unsafe. >>>>>>> >>>>>>> The fix extracts the inner part of the loop into a separate function >>>>>>> where the result of each step is properly checked for pending >>>>>>> exceptions. If there is a pending exception the loop will be >>>>>>> interrupted, resources cleaned and >>>>>>> Java_sun_tracing_dtrace_JVM_activate0() function will return 0 >>>>>>> with the >>>>>>> pending exception recorded in env. >>>>>>> >>>>>>> >>>>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/8030115/webrev.00 >>>>>>> >>>>>>> Thanks, >>>>>>> >>>>>>> -JB- >>>>>> >>>>>> >>>>> >>>> >>> >>> >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
