On Wed, 18 Jun 2025 11:52:17 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: > > Add a byte buffer version and rename parameters. test/lib/RedefineClassHelper.java line 87: > 85: } > 86: return 0; > 87: } I understand, that this is copy of the old code, but this method does not look good. - it would be better to return -1 if the string not found (0 is valid offset) - calculation of the buf's upper bound is incorrect when offset > 0 I'd suggest to update it to something like (not tested, also need to update callers to compare with -1) private static int getStringIndex(String needle, byte[] buf, int offset) { byte[] needleArray = needle.getBytes(StandardCharsets.US_ASCII); for (int i = offset; i < buf.length - needleArray.length; i++) if (Arrays.equals(buf, offset, offset + needleArray.length, needleArray, 0, needleArray.length)) { return offset; } } return -1; } ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25857#discussion_r2162549862