On Fri, 21 Nov 2025 12:35:01 GMT, Jorn Vernee <[email protected]> wrote:
>> Liam Miller-Cushon has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Improve test coverage, and more fixes
>> - Review feedback
>>
>> * document assertion to link to bytesCompatible
>> * throw IAE for length > Integer.MAX_VALUE
>> * javadoc fixes
>
> test/jdk/java/foreign/TestStringEncoding.java line 108:
>
>> 106: @Test(dataProvider = "strings")
>> 107: public void testStringsLength(String testString) {
>> 108: Set<String> excluded = Set.of("yen");
>
> I know the yen character is translated to `/` in some encodings when doing a
> round trip. Maybe we should just avoid this issue by switching it out for
> e.g. `"section \u00A7"`, which is `ยง` and doesn't have the same problem.
Thanks, sounds good! Done
> test/jdk/java/foreign/TestStringEncoding.java line 180:
>
>> 178: assertThrows(IndexOutOfBoundsException.class, () ->
>> 179: MemorySegment.copy(testString,
>> StandardCharsets.UTF_8, 0, text, 0, testString.length() + 1));
>> 180: // dstOffset > byteSize() + B
>
> Suggestion:
>
> // dstOffset > byteSize() - B
Thanks for the catch, fixed
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28043#discussion_r2549704610
PR Review Comment: https://git.openjdk.org/jdk/pull/28043#discussion_r2549709383
