RFR: 8321971: Improve the user name detection logic in perfMemory get_user_name_slow

Johan Sjölen jsjolen at openjdk.org
Mon Dec 18 10:13:40 UTC 2023


On Thu, 14 Dec 2023 10:13:51 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

> Can I please get a review of this change which proposes to improve the code in `get_user_name_slow` function, which is used to identify the target JVM owner's user name? This addresses https://bugs.openjdk.org/browse/JDK-8321971.
> 
> As noted in that JBS issue, in its current form, the nested loop ends up iterating over the directory contents of `hsperfdata_xxx` directory and then for each iteration it checks if the name of the entry matches the pid. This iteration shouldn't be needed and instead one could look for a file named `<pid>` within that directory.
> 
> No new test has been added, given the nature of this change. Existing tier1, tier2, tier3 and svc_tools tests pass with this change on Linux, Windows and macosx.

Hi,

A small review on the usage of the string functions. Have you considered using `stringStream` and passing references to an instance of it instead?

src/hotspot/os/posix/perfMemory_posix.cpp line 447:

> 445: 
> 446:   return name;
> 447: }

This drops the `snprintf` return value which indicates if an error has occurred, can this be remediated?

src/hotspot/os/posix/perfMemory_posix.cpp line 617:

> 615: 
> 616:       if (statbuf.st_ctime > oldest_ctime) {
> 617:         char* user = strchr(dentry->d_name, '_') + 1;

Invalid pointer if `strchr` returns null.

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

PR Review: https://git.openjdk.org/jdk/pull/17104#pullrequestreview-1786373167
PR Review Comment: https://git.openjdk.org/jdk/pull/17104#discussion_r1429831911
PR Review Comment: https://git.openjdk.org/jdk/pull/17104#discussion_r1429840030


More information about the hotspot-runtime-dev mailing list