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

Resolve file descriptor names #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

kirelagin
Copy link
Contributor

@kirelagin kirelagin commented Jun 14, 2019

This is somewhat WIP, as it prints file paths unconditionally (i.e. no -y flag). Not sure how bad we want it.


This change is Reviewable

This commit implement a C function that maps (pid, fd) -> path and works
both on Linux and macOS.

The following syscalls are augmented with the path of fd involved:

* read
* write
* close

Not yet implemented:

* openat as it needs special support for CWD
* all other syscalls that do not have details yet :)
Copy link
Owner

@nh2 nh2 left a comment

Choose a reason for hiding this comment

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

Huh, looks like I hadn't clicked submit here yet at ZuriHac, sorry for that!

Some questions added

#include <memory.h>
#endif

// assert: sizeof(buf) == MAXPATHLEN
Copy link
Owner

Choose a reason for hiding this comment

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

This precondition is cheap to check, should we check it?

throwErrnoIfMinus1 "resolve_fd_name" (c_resolve_fd_name pid fd ptr) >>= \case
0 -> Just <$> peekCString ptr
1 -> pure Nothing
_ -> throwIO $ AssertionFailed "resolveFdName: something terrible happened"
Copy link
Owner

Choose a reason for hiding this comment

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

Doesn't 2 mean "OS not supported"?

Perhaps we should explicitly say that?

#if defined __linux__

char proc_path[MAXPATHLEN];
snprintf(proc_path, MAXPATHLEN, "/proc/%d/fd/%d", pid, fd);
Copy link
Owner

Choose a reason for hiding this comment

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

We might check for errors by checking the return value of snprintf.

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