On Wed, 3 Dec 2025 03:10:19 GMT, Serguei Spitsyn <[email protected]> wrote:
>> Sergey Chernyshev has updated the pull request with a new target base due to >> a merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains eight additional >> commits since the last revision: >> >> - Merge branch 'master' into JDK-8319589 >> - Merge branch 'master' into JDK-8319589 >> - Removed unused native method >> - addressed review comment #2 >> - Apply suggestions from code review >> >> Co-authored-by: David Holmes >> <[email protected]> >> - Update >> src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java >> >> Co-authored-by: Andrey Turbanov <[email protected]> >> - addressed review comments >> - 8319589: Attach from root to a user java process not supported in Mac > > src/hotspot/os/bsd/os_bsd.cpp line 881: > >> 879: uid_t uid = get_process_uid(pid); >> 880: return (uid != (uid_t)-1) ? os::Posix::is_root(uid) : false; >> 881: } > > Nit: > - missed dot at the end of comments at lines 864, 877 > - indent above has to be 2, not 4 Thanks @sspitsyn , fixed. > src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java line > 78: > >> 76: // Then we attempt to find the socket file again. >> 77: // In macOS the socket file is located in per-user temp >> directory. >> 78: String tempDir = getTempDirFromPid(pid); > > Nit: I'd suggest to rename it to `tempdir` to follow the local naming > convention which prevails. Thanks! Fixed. > src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java line > 223: > >> 221: } >> 222: >> 223: private File createAttachFile(String dir, int pid) throws >> IOException { > > Nit: I'd suggest to rename `dir` to `tmpdir` to keep it unified. done. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2583859087 PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2583860448 PR Review Comment: https://git.openjdk.org/jdk/pull/25824#discussion_r2583860781
