On Fri 08 Aug 2014 12:09:20 [email protected] 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
signature.asc
Description: This is a digitally signed message part.
------------------------------------------------------------------------------
_______________________________________________ Strace-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/strace-devel
