On Fri, 10 Jul 2026 07:15:26 GMT, Serguei Spitsyn <[email protected]> wrote:
>> Coleen Phillimore has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Moved down AltTempDir in jinfo.md
>
> src/hotspot/os/linux/os_linux.cpp line 1562:
>
>> 1560: // Since the attach mechanism uses the socket name length, this
>> severely limits the length of the
>> 1561: // alternate temporary directory name. We don't check that here since
>> the temporary directory is
>> 1562: // used for many things. The perfData and attach code will check it.
>
> Nit: I can't fully understand the comment at lines 1557-1562.
> Are we trying to say this:
> - absolute paths are enforced
> - existence and writeability is warned but not rejected ?
>
> Is it possible to simplify the wording a little bit to make it more clear?
Okay, I'll try to make it clearer.
> src/jdk.internal.jvmstat/linux/classes/sun/jvmstat/PlatformSupportImpl.java
> line 175:
>
>> 173: /*
>> 174: * Extract the VM ID (PID) from a file path.
>> 175: * Specifically the host pid for a container process.
>
> Nit: Inconsistency with use of `PID` and `pid`. Better to have the same
> naming style to avoid confusions.
Fixed to say pid.
> test/jdk/com/sun/tools/attach/JvmTempDirTest.java line 149:
>
>> 147: System.out.println("Started after " + elapsedTime + "s");
>> 148:
>> 149: final String pidFile = "JvmTempDirTest.Application.pid-" +
>> counter++;
>
> Nit: I do not see where this string is really used. But this part `counter++`
> can be needed.
It is not used.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31407#discussion_r3570418726
PR Review Comment: https://git.openjdk.org/jdk/pull/31407#discussion_r3570465206
PR Review Comment: https://git.openjdk.org/jdk/pull/31407#discussion_r3570477095
