[PATCH] Print absolute paths in printpathn when -yy is used

[Dmitry V. Levin]

Hi Zubin,

On Thu, Jun 05, 2014 at 04:49:02PM +0530, zubin.mithra at gmail.com wrote:
[...]
> @@ -646,6 +646,8 @@ extern const char *signame(int);
>  extern void pathtrace_select(const char *);
>  extern int pathtrace_match(struct tcb *);
>  extern int getfdpath(struct tcb *, int, char *, unsigned);
> +extern int get_tracee_cwd(struct tcb *, char *);
> +extern bool print_abspath(struct tcb *, char *, int);

Please do not export these functions unless you are going to use them
elsewhere.

> --- a/util.c
> +++ b/util.c
> @@ -571,7 +571,9 @@ void
>  printpathn(struct tcb *tcp, long addr, int n)
>  {
>  	char path[MAXPATHLEN + 1];
> +	char cwd[MAXPATHLEN + 1];
>  	int nul_seen;
> +	int valid_length = 1;

Please move these new objects to the branch where they are going to be used.

btw, your get_tracee_cwd may write up to MAXPATHLEN+2 bytes.

>  	if (!addr) {
>  		tprints("NULL");
> @@ -593,8 +595,28 @@ printpathn(struct tcb *tcp, long addr, int n)
>  		n++;
>  		outstr = alloca(4 * n); /* 4*(n-1) + 3 for quotes and NUL */
>  		string_quote(path, outstr, -1, n);
> +
> +		/* If -yy is used and the path is not absolute, find the cwd of the tracee
> +		   and print that out first */
> +		if (show_fd_path == 2 && outstr[1] != '/') {

1. show_fd_path > 1
2. path[0] != '/' (think of strace -xx)

> +			/* If the call to get_tracee_cwd succeeds, we concatenate the
> +			   cwd and outstr and print it out. If the call fails, we just
> +			   print out the relative path. */
> +			char *outcwd;
> +			int cwd_len;
> +			cwd_len = get_tracee_cwd(tcp, cwd);
> +			if (cwd_len != -1) {
> +				outcwd = alloca(4 * cwd_len);

4 * cwd_len may be not enough.

> +				string_quote(cwd, outcwd, -1, cwd_len+1);
> +				outcwd[strlen(outcwd)-1] = '\0';

The string made by string_quote is already null-terminated.

> +				tprintf("%s", outcwd);

Please use tprints for this.

> +				if (strlen(cwd) + strlen(outstr) > MAXPATHLEN)
> +					valid_length = 0;
> +			}
> +		}
> +		if (show_fd_path == 2) outstr += 1;
>  		tprints(outstr);
> -		if (!nul_seen)
> +		if (!nul_seen || (show_fd_path==2) && !valid_length)
>  			tprints("...");

I don't see why our decision on printing trailing dots should depend on
get_tracee_cwd.

I think it would be better just to print cwd prefix if required conditions
are met:

		char *outstr;

		path[n] = '\0';

		if (show_fd_path > 1 && *path && *path != '/')
			print_tracee_cwd(tcp);

		n++;
		outstr = alloca(4 * n); /* 4*(n-1) + 3 for quotes and NUL */
		string_quote(path, outstr, -1, n);
		tprints(outstr);
		if (!nul_seen)
			tprints("...");

> @@ -1549,3 +1571,29 @@ clearbpt(struct tcb *tcp)
>  	tcp->flags &= ~TCB_BPTSET;
>  	return 0;
>  }
> +
> +/* Return the current working directory of the tracee process,
> +   with a trailing slash. */
> +int
> +get_tracee_cwd(struct tcb *tcp, char *cwd)
> +{
> +	int link_size = sizeof("/proc/%u/cwd") + sizeof(int) * 3;
> +	char linkpath[link_size];
> +	ssize_t n;
> +
> +	snprintf(linkpath, link_size, "/proc/%u/cwd", tcp->pid);
> +	n = readlink(linkpath, cwd, MAXPATHLEN);
> +
> +	/* If readlink fails, something is abnormal(path > 4096) about
> +	   the traced process and its cwd, so return an empty cwd. */
> +	if (n == -1) {
> +		cwd[0] = '\0';
> +		return n;
> +	}
> +
> +	if (n >= 0) {
> +		cwd[n] = '/';
> +		cwd[n+1] = '\0';
> +	}
> +	return n+1;
> +}

(n > 0) is the only case when we have a cwd prefix to print,
(n == 0) is even more odd than (n < 0).


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20140611/96fd05cd/attachment.bin>