RFC: path display and path filtering features.

Roland McGrath roland at redhat.com
Tue Feb 15 00:53:54 UTC 2011


>  1) Support for non-Linux OSes isn't done yet.  For example, the
>     handling of the clone system call where it is determined whether
>     to copy or share the descriptor state assumes the OS is Linux.

Maintenance of strace has been thoroughly Linux-centric for years now.
I really think it's just fine for a new feature to be entirely under
#ifdef LINUX and to let people concerned with other systems worry about
it when they want to put the effort into it.

>  2) It is assumed that TRACE_DESCRIPTOR means arg[0] is a file
>     descriptor, and that TRACE_FILE means arg[0] is path.  That's not
>     100% accurate, but so far it's close enough for all of my use
>     cases.
[...]
>  6) There are probably some more obsure ways to associate an fd with a
>     path that I haven't caught (for example, the fcntl DUPFD operations).

These are to me the same item, under the general heading of how the
fd-tracking is tied into syscall decoding.  I don't think this short-cut is
adequate, and I also don't see the need to have done it that way at all.

You already have individual hooks in the specific syscall decoders where an
fd is presented.  So why do you need a pre-pass that operates blindly on
all system calls?  The same calls that decode an fd for printing can be
where the tie into the tracking happens, can't they?  

The only generic case I really see is that return-value printing is generic
in the way strace is structured now, so when the return value is an fd, you
need to handle that.  That seems like a good fit for a flag bit in the
sysent table (saying the return value is an fd being established).

Hmm, perhaps this is meant so that the fd-tracking happens on the calls
that are not being printed at all due to the use of -e options?

>  3) The code that tracks fd table state is invoked even if paths are
>     neither being displayed nor filtered-upon.  That means that the
>     handling of a few system calls (e.g. open/close/dup) takes
>     slightly longer than it needs to in the cases where neither of the
>     two new options is being used.

That should be fixed and it certainly ought to be easy and clean to
conditionalize the extra work centrally.

>  4) I have no idea how portable the readlink() on /proc/<pid>/fd/<fd>
>     mechanism is, or if there are alternative mechanisms for other OSes.

It is entirely Linux-specific, but that is fine.  If other systems have a
mechanism, people interested in their support can write the code.  Just
make the code use clean divisions into subroutines as good practice would
dictate regardless, and this is ready for future porting.

>  5) The indentation needs to be fixed.  I ran all the sources through
>     indent in order to get them consistent before I started work, and
>     the attached diffs were done after everything was re-indented.

That's just not the way things are done.  The prevailing formatting
conventions of the strace source are not my first choice either, but
we leave things how they have been and make new code match.

>  7) Though descriptors with the close-on-exec flag set aren't handled
>     properly, the path will get replaced when the child process does
>     associate a new file with the file descriptor.

I think that's fine--perhaps even a feature.  That is, it seems worthwhile
to see the last-known association of an fd when it's used in a later call
that fails with EBADF.  

Ideally, you the fd-tracking would notice FD_CLOEXEC being set or cleared
in an fcntl call, and the O_CLOEXEC flag being used in an establishing call
like open.  This seems fairly straightforward: it's just tracking a flag
bit along with tracking a file name--it also seems wortwhile to track the
open modes (O_RDONLY vs O_RDWR, etc.)  It's then pretty trivial to keep an
"exec generation" count in a tcb and its fd-tracking records, so you can
report the known details about an fd as "last opened as file foo but then
closed implicitly by exec of bar".  That's desireable just the same as is
reporting "last opened as file foo but then closed by a close call".

> If, with some additional work, the new options would be accepted, then
> I'd be happy to work on addressing these (or other) issues.

These days, I'm not really a gatekeeper of patches, that's mostly Dmitry.
But this seems like useful stuff to me.  So, I would say it is worth
working on it to make it worthy of including.  The current issues, both
trivia like indentation, and substance like lack of correct identification
of fd and file name parameters/results in all calls, make it not yet worthy.


Thanks,
Roland




More information about the Strace-devel mailing list