On Fri, 12 Jul 2024 21:03:26 GMT, Alex Menkov <amen...@openjdk.org> wrote:

>> The change fixes a bug in Attach API implementation on Windows when 
>> argument(s) are longer than 1023 bytes
>> 
>> testing: test/hotspot/jtreg/serviceability/attach, 
>> test/jdk/com/sun/tools/attach on Oracle supported platforms
>
> Alex Menkov has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   feedback

test/hotspot/jtreg/serviceability/attach/LongArgTest.java line 79:

> 77:         // Value length exceeds 1K.
> 78:         Test withLongValue() {
> 79:             flagValue = generateValue(1024 + 1);

Shouldn't we also be testing exactly 1024 and expect it to work?

test/hotspot/jtreg/serviceability/attach/LongArgTest.java line 98:

> 96: 
> 97:                 if (!flagValue.equals(actualValue)) {
> 98:                     String msg = "Actual values is different: "

Suggestion:

                    String msg = "Actual value is different: "

test/hotspot/jtreg/serviceability/attach/LongArgTest.java line 159:

> 157:     private static Test test(LingeredApp app) {
> 158:         return new Test(app);
> 159:     }

I think the method would be better off placed right after main().

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19780#discussion_r1676583710
PR Review Comment: https://git.openjdk.org/jdk/pull/19780#discussion_r1676427158
PR Review Comment: https://git.openjdk.org/jdk/pull/19780#discussion_r1676427832

Reply via email to