On Wed, 28 Feb 2024 01:18:50 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
> Please review this change that renames some test .h files to .hpp. These > files contain C++ code and should be named accordingly. Some of them contain > uses of NULL, which we change to nullptr. > > The renamed files are: > > test/hotspot/jtreg/vmTestbase/nsk/share/aod/aod.h > test/hotspot/jtreg/vmTestbase/nsk/share/jni/jni_tools.h > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/jvmti_tools.h > test/hotspot/jtreg/vmTestbase/nsk/share/jvmti/JVMTITools.h > test/hotspot/jtreg/vmTestbase/nsk/share/native/nsk_tools.h > > test/lib/jdk/test/lib/jvmti/jvmti_thread.h > test/lib/jdk/test/lib/jvmti/jvmti_common.h > test/lib/native/testlib_threads.h > > The #include updates were performed mostly mechanically, and builds would fail > if there were mistakes. The exception is test > test/hotspot/jtreg/serviceability/jvmti/FollowReferences/FieldIndices/libFieldIndicesTest.cpp, > which was added after I'd done the mechanical update, so was updated by hand. > > The copyright updates were similarly performed mechanically. All but a handful > of the including files have already had their copyrights updated recently, > likely as part of JDK-8324681. > > Thus, the only "interesting" changes are to the renamed files. > > Testing: mach5 tier1 I asked for these to be done together since it's the same scrolling for each of these. I added suggestions, but please feel free to ignore them since it would be prudent pull down and to rebuild after making them and it's not worth it. test/hotspot/jtreg/serviceability/jvmti/DynamicCodeGenerated/libDynamicCodeGenerated.cpp line 25: > 23: > 24: #include <string.h> > 25: #include <jvmti.h> Is jvmti.h a C file? Sometimes it has <> sometimes it's included with "". I don't expect to see a change. I was just curious. test/hotspot/jtreg/serviceability/jvmti/vthread/FollowReferences/libVThreadStackRefTest.cpp line 26: > 24: #include <jni.h> > 25: #include <jvmti.h> > 26: #include <jvmti_common.hpp> Should this be in quotes rather than <> ? test/hotspot/jtreg/serviceability/jvmti/vthread/GetThreadStateMountedTest/libGetThreadStateMountedTest.cpp line 26: > 24: #include <jni.h> > 25: #include <jvmti.h> > 26: #include <jvmti_common.hpp> Another. test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetClassStatus/getclstat007/getclstat007.cpp line 28: > 26: #include "jvmti.h" > 27: #include "agent_common.hpp" > 28: #include "JVMTITools.hpp" There's a lower case jvmti_tools.hpp and an uppercase JVMTITools.hpp now. Maybe someone could do a cleanup of these tests (please!) test/hotspot/jtreg/vmTestbase/nsk/jvmti/RetransformClasses/retransform004/retransform004.cpp line 29: > 27: #include <jvmti.h> > 28: #include "agent_common.hpp" > 29: #include <jvmti_tools.hpp> Suggestion: #include "jvmti_tools.hpp" ------------- Marked as reviewed by coleenp (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/18035#pullrequestreview-1906370538 PR Review Comment: https://git.openjdk.org/jdk/pull/18035#discussion_r1506026145 PR Review Comment: https://git.openjdk.org/jdk/pull/18035#discussion_r1506031049 PR Review Comment: https://git.openjdk.org/jdk/pull/18035#discussion_r1506031853 PR Review Comment: https://git.openjdk.org/jdk/pull/18035#discussion_r1506038573 PR Review Comment: https://git.openjdk.org/jdk/pull/18035#discussion_r1506045478
