[PATCHv4 1/2] Support unix sockets in -yy option

[Dmitry V. Levin]

On Wed, Dec 24, 2014 at 08:59:31PM +0900, Masatake YAMATO wrote:
> This patch extends -yy option; the peer address of a unix socket can be
> printed like an inet socket.
[...]
> +static bool
> +unix_parse_response(const void *data, int data_len, const unsigned long inode)
> +{
> +	const struct unix_diag_msg *diag_msg = data;
> +	int rta_len = data_len - NLMSG_LENGTH(sizeof(*diag_msg));
> +	struct rtattr *attr;

I've swapped these two lines, and moved "uint32_t peer" definition after
them.  Even if compiler is capable of rearranging automatic variables for
proper alignment, code usually looks better when variables are already
aligned properly.

> +	size_t path_len = 0;
> +	char path[UNIX_PATH_MAX + 1] = { [0] = '\0' };

I'm not sure why you changed the way how "path" is initialized.
Your fist edition was OK, so I've changed it back.

> +	uint32_t peer = 0;
> +	if (diag_msg->udiag_ino != inode)
> +		return false;
> +	if (diag_msg->udiag_family != AF_UNIX)
> +		return false;
> +
> +	attr = (struct rtattr*) (diag_msg +1);
> +	while (RTA_OK(attr, rta_len)) {

BTW, this looks like a classic "for" expression.

> +		switch (attr->rta_type) {
> +		case UNIX_DIAG_NAME:
> +			if (!path_len) {
> +				path_len = RTA_PAYLOAD(attr);
> +				if (path_len > UNIX_PATH_MAX)
> +					path_len = UNIX_PATH_MAX;
> +				memcpy(path, RTA_DATA(attr), path_len);

There was a null-termination in your first edition, I've put it back.

> +				break;
> +			}

This is probably a typo, "break" statement is needed in both cases.
Fixed.

> +		case UNIX_DIAG_PEER:
> +			if (RTA_PAYLOAD(attr) >= 4)
> +				peer = *(uint32_t *)RTA_DATA(attr);
> +			break;
> +		}
> +		attr = RTA_NEXT(attr, rta_len);
> +	}
> +
> +	/* prints the getting information with following format:
> +	 * "UNIX:[" SELF_INODE [ "->" PEER_INODE ][ "," SOCKET_FILE ] "]" */
> +	if (peer || path_len) {
> +		tprintf("UNIX:[%lu", inode);
> +		if (peer)
> +			tprintf("->%u", peer);
> +		if (path_len) {
> +			if (path[0] == '\0')
> +				tprintf(",@\"%s\"", path + 1);
> +			else
> +				tprintf(",\"%s\"", path);
> +		}

When I mentioned printsock(), I meant that printsock() uses string_quote():

			if (path[0] == '\0') {
				char *outstr = alloca(4 * path_len - 1);
				string_quote(path + 1, outstr, -1, path_len);
				tprintf(",@%s", outstr);
			} else {
				char *outstr = alloca(4 * path_len + 3);
				string_quote(path, outstr, -1, path_len + 1);
				tprintf(",%s", outstr);
			}


No need to submit v5, I've applied your patch with these corrections.
Thanks!


-- 
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/20141227/1d251d27/attachment.bin>