[PATCH v4] Print ip and port associated with descriptor with -yy

[Mike Frysinger]

On Fri 08 Aug 2014 12:09:20 zubin.mithra at gmail.com wrote:
> --- a/defs.h
> +++ b/defs.h
> @@ -67,6 +67,11 @@
>  #include <time.h>
>  #include <sys/time.h>
>  #include <sys/syscall.h>
> +#include <netinet/in.h>
> +#include <sys/socket.h>
> +#include <arpa/inet.h>
> +#include <linux/netlink.h>
> +#include <linux/inet_diag.h>

strace isn't linux specific, so any linux-specific code has to be behind LINUX 
checks

> +int
> +parse_response(struct inet_diag_msg *diag_msg, int inodenr) {

uncuddle the brace for funcs

> +	char remote_addr_buf[INET6_ADDRSTRLEN];
> +	int rport;
> +
> +	if (diag_msg->idiag_inode != inodenr)
> +		return -1;
> +
> +	memset(remote_addr_buf, 0, sizeof(remote_addr_buf));

do you really need to clear the whole thing every time ?  can't you check 
return values of inet_ntop below instead ?

> +int
> +send_query(int sockfd, int proto, int family) {
> +	struct msghdr msg;
> +	struct nlmsghdr nlh;
> +	struct inet_diag_req_v2 conn_req;
> +	struct sockaddr_nl sa;
> +	struct iovec iov[4];

down below you use iov[0] and iov[1] ... do you need [2] and [3] ?

> +	memset(&msg, 0, sizeof(msg));
> +	memset(&sa, 0, sizeof(sa));
> +	memset(&nlh, 0, sizeof(nlh));
> +	memset(&conn_req, 0, sizeof(conn_req));

do you need to zero these out when you initialize their content below ?

> +
> +/* Given an inode number of a socket, print out the details
> + * of the remote ip address and remote port */
> +int
> +printsockdetails(int inodenr)
> +{
> +	int sockfd;
> +	int i, j;
> +	int protocols[] = {IPPROTO_TCP, IPPROTO_UDP};
> +	int families[] = {AF_INET, AF_INET6};
> +	int plen = sizeof(protocols)/sizeof(protocols[0]);
> +	int flen = sizeof(families)/sizeof(families[0]);

use ARRAY_SIZE

i think i/j/plen/flen should be size_t

> +	//Create the monitoring socket

we use /* Comments. */

> +	if((sockfd = socket(AF_NETLINK, SOCK_DGRAM, NETLINK_INET_DIAG)) == -1)

split the assignment out to a line by itself

> +		return -1;
> +
> +	for (i = 0; i < plen; i++) {
> +		for (j = 0; j < flen; j++) {
> +			if (send_query(sockfd, protocols[i], families[j]) < 0) {
> +				close(sockfd);
> +				return -1;
> +			}
> +			if (parse_responses(sockfd, inodenr) == 0) {
> +				close(sockfd);
> +				return 0;
> +			}
> +		}
> +	}
> +	close(sockfd);
> +	return -1;

i'd set up a return value, set it to -1 at the top of the loop, and then only 
set it to 0 when things succeed.  then the add a label to the close/return at 
the end, and change the other close/return statements to a goto.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20140809/5b8065eb/attachment.bin>