Skip to content

DAOS-18563 control: Find parent process name more flexibly#17494

Open
mjmac wants to merge 1 commit intomasterfrom
mjmac/DAOS-18563
Open

DAOS-18563 control: Find parent process name more flexibly#17494
mjmac wants to merge 1 commit intomasterfrom
mjmac/DAOS-18563

Conversation

@mjmac
Copy link
Contributor

@mjmac mjmac commented Feb 3, 2026

In some cases, it's more reliable to get the process name from
/proc/$pid/comm, so prefer that with a fallback to the old method
of using readlink on /proc/$pid/exe.

Signed-off-by: Michael MacDonald github@macdonald.cx

@github-actions
Copy link

github-actions bot commented Feb 3, 2026

Ticket title is 'Check /proc/pid/comm first when getting process name'
Status is 'In Review'
https://daosio.atlassian.net/browse/DAOS-18563

@daosbuild3
Copy link
Collaborator

@daosbuild3
Copy link
Collaborator

@daosbuild3
Copy link
Collaborator

@mjmac mjmac force-pushed the mjmac/DAOS-18563 branch 2 times, most recently from f4a6d8d to c058976 Compare February 11, 2026 20:38
@mjmac mjmac marked this pull request as ready for review February 13, 2026 18:01
@mjmac mjmac requested review from a team as code owners February 13, 2026 18:01
@mjmac mjmac requested a review from kjacque February 13, 2026 18:01
Copy link
Contributor

@kjacque kjacque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, comment is just a suggestion.

}

return filepath.Base(pPath), nil
return string(data[:len(data)-1]), nil // trim trailing newline
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could use strings.Trim which might eliminate the need for the comment

@mjmac mjmac requested a review from knard38 February 17, 2026 17:57
In some cases, it's more reliable to get the process name from
/proc/$pid/comm, so prefer that with a fallback to the old method
of using readlink on /proc/$pid/exe.

Signed-off-by: Michael MacDonald <github@macdonald.cx>
Copy link
Contributor

@knard38 knard38 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some concerns with using /proc/<pid>/comm: see my comments.

func (p *Process) ParentProcessName() (string, error) {
pPath, err := os.Readlink(fmt.Sprintf("/proc/%d/exe", os.Getppid()))
if err != nil {
data, err := os.ReadFile(fmt.Sprintf("/proc/%d/comm", os.Getppid()))
Copy link
Contributor

@knard38 knard38 Feb 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my understanding, using /proc/<pid>/comm is not reliable: if the name of the process is greater than 16 characters, it will be truncated. From my side, I prefer to process /proc/<pid>/cmdline system file which contains the full name of the executable and the arguments.

Copy link
Contributor

@tanabarr tanabarr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@knard38 objection seems reasonable, can we make the switch?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Comments