Dmitry, Please, also change
!strcmp(...) to strcmp(...) == 0 because semantically strcmp result is not a boolean false. -Dmitry On 2016-09-20 06:38, serguei.spit...@oracle.com wrote: > Hi Dmitry, > > > Thanks a lot for this additional test coverage and discovering new bug: > https://bugs.openjdk.java.net/browse/JDK-8165681 > > The tests look pretty good to me. > > A couple of minor comments. > > Dots are missed in the .c files comments. > > > http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/test/serviceability/jvmti > > > > 265 printf("Expecting to find '%s' class in ClassLoad events > duringVM start phase.\n", EXPECTED_SIGNATURE); > 266 if (class_in_class_load_events_vm_start == JNI_FALSE) { > 267 throw_exc(env, "Unable to find expected class in > ClassLoad events duringstart phase!\n"); > 268 return FAILED; > 269 } > 270 > 271 if (class_prepare_events_vm_start_count == 0) { > 272 throw_exc(env, "Didn't get ClassPrepare events in start > phase!\n"); > 273 return FAILED; > 274 } > 275 > 276 printf("Expecting to find '%s' class in ClassPrepare events > duringVM start phase.\n", EXPECTED_SIGNATURE); > 277 if (class_in_class_prepare_events_vm_start == JNI_FALSE) { > 278 throw_exc(env, "Unable to find expected class in > ClassPrepare events duringstart phase!\n"); > 279 return FAILED; > 280 } > > > Could you, please, replace "start phase" with "early start phase"? > It will be more clear that the start phase mode is "early". > > 288 if (class_in_class_prepare_events_vm_start == JNI_TRUE) { > 289 throw_exc(env, "Class is found in ClassLoad events > duringVM Start phase!\n"); > 290 return FAILED; > 291 } > 292 > 293 printf("Expecting that '%s' class is absent in ClassPrepare > events.\n", EXPECTED_SIGNATURE); > 294 if (class_in_class_prepare_events_vm_start == JNI_TRUE) { > 295 throw_exc(env, "Class is found in ClassPrepare events > duringVM Start phase!\n"); > 296 return FAILED; > 297 } > > Could you, please, replace "VM Start phase" with "normal VM start > phase"? It will be more clear that the start phase mode is "normal". > Thanks, Serguei On 9/19/16 05:47, Dmitry Dmitriev wrote: >> Hello, Please review new tests for module aware agents. There are 3 >> tests: 1) MAAClassFileLoadHook.java - verifies ClassFileLoadHook event >> with different combinations of can_generate_early_vmstart and >> can_generate_early_class_hook_events JVMTI capabilities. Expects to >> find(or not) class from java.base module in the right VM phase. 2) >> MAAClassLoadPrepare.java - verifies ClassLoad and ClassPrepare events >> with and without can_generate_early_vmstart JVMTI capability. Expects >> to find(or not) class from java.base module in the VM start phase. 3) >> MAAThreadStart.java - verifies ThreadStart event with >> can_generate_early_vmstart JVMTI capability. Expect to find events in >> the VM start phase. JBS: >> https://bugs.openjdk.java.net/browse/JDK-8150758 webrev.00: >> http://cr.openjdk.java.net/~ddmitriev/8150758/webrev.00/ >> <http://cr.openjdk.java.net/%7Eddmitriev/8150758/webrev.00/> Testing: >> RBT on all platforms Thanks, Dmitry -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
