On Fri, 27 Jun 2025 11:37:30 GMT, Coleen Phillimore <cole...@openjdk.org> wrote:
>> I copied this code for another test in the Valhalla repo and thought it
>> would be a good utility function. It might be better written using the
>> Classfile API.
>> Tested with test.
>
> Coleen Phillimore has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Rename replaceAllStrings to replaceClassName and add back the byte buffer
> version of the function.
Changes requested by dholmes (Reviewer).
test/hotspot/jtreg/serviceability/jvmti/RedefineClasses/ClassVersionAfterRedefine.java
line 56:
> 54: // redefining the original version of "TestClassXXX" (i.e.
> "TestClassOld").
> 55: buf = RedefineClassHelper.replaceClassName(cvar, "TestClassNew",
> "TestClassXXX");
> 56: // Now redine the original version of "TestClassXXX" (i.e.
> "TestClassOld").
Suggestion:
// Now redefine the original version of "TestClassXXX" (i.e.
"TestClassOld").
Pre-existing
test/lib/RedefineClassHelper.java line 85:
> 83: * @param bytes in original class file.
> 84: * @param oldClassName old class name.
> 85: * @param newClassName new class name to replace with old class name.
Suggestion:
* @param newClassName new class name to replace the old class name.
test/lib/RedefineClassHelper.java line 87:
> 85: * @param newClassName new class name to replace with old class name.
> 86: */
> 87: public static byte[] replaceClassName(byte[] bytes, String
> oldClassName, String newClassName) throws Exception {
Just realized that `oldClassName` is not actually used. Suggestion:
/*
* Copy the class defined by `bytes`, replacing the name of the class with
`newClassName`, so that both
* old and new classes can be compiled by jtreg for the test.
*
* @param bytes the bytes presenting the original classfile
* @param newClassName the new class name for the returned class representation
* @ return a copy of the class represented by `bytes` but with the name
`newClassName`
*/
public static byte[] replaceClassName(byte[] bytes, String newClassName)
throws Exception {
-------------
PR Review: https://git.openjdk.org/jdk/pull/25857#pullrequestreview-2969920311
PR Review Comment: https://git.openjdk.org/jdk/pull/25857#discussion_r2174234229
PR Review Comment: https://git.openjdk.org/jdk/pull/25857#discussion_r2174235361
PR Review Comment: https://git.openjdk.org/jdk/pull/25857#discussion_r2174242441
