Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Proc.Cwd() and Proc.RootDir() #109

Merged
merged 1 commit into from
Sep 20, 2018
Merged

Conversation

pas2k
Copy link
Contributor

@pas2k pas2k commented Sep 19, 2018

Addressing @grobie, as per contributing guidelines for trivial improvement.

The PR implements reading /proc/pid/cwd and /proc/pid/root similar to how /proc/pid/exe is implemented.

Naming is inspired by https://talks.golang.org/2014/names.slide and golang standard library.

Copy link
Member

@grobie grobie left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!

Please add some minimal tests. Have a look at the existing test setup and fixtures files. Make sure to use an example from a live sytem (some procfs files include null bytes, other trailing newlines), we might want to format the returned value.

proc.go Outdated
@@ -156,6 +156,26 @@ func (p Proc) Executable() (string, error) {
return exe, err
}

// Cwd returns the absolute path to the current working directory of the process.
func (p Proc) Cwd() (string, error) {
exe, err := os.Readlink(p.path("cwd"))
Copy link
Member

Choose a reason for hiding this comment

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

Please rename the variable to cwd or at least a generic v.

proc.go Outdated

// RootDir returns the absolute path to the process's root directory (as set by chroot)
func (p Proc) RootDir() (string, error) {
exe, err := os.Readlink(p.path("root"))
Copy link
Member

Choose a reason for hiding this comment

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

Please rename the variable to root or at least a generic v.

Signed-off-by: Pavel Khlebovich <pas.nteg@gmail.com>
@pas2k
Copy link
Contributor Author

pas2k commented Sep 19, 2018

Got it. Nice fixture system, by the way.

Since the introduced functions only operate on symlinks, the returned data should be as reliable as the one returned by os.Readlink(). Tests are included for the missing, broken and valid symlinks.

Copy link
Member

@grobie grobie left a comment

Choose a reason for hiding this comment

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

Thanks! I was missing that these are just symlinks.

@grobie grobie merged commit 418d78d into prometheus:master Sep 20, 2018
remijouannet pushed a commit to remijouannet/procfs that referenced this pull request Oct 20, 2022
Signed-off-by: Pavel Khlebovich <pas.nteg@gmail.com>
bobrik pushed a commit to bobrik/procfs that referenced this pull request Jan 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants