[iproute PATCH 00/18] ss: Minor code review

[iproute PATCH 00/18] ss: Minor code review

[Phil Sutter]

This is a series of misc changes to ss code which happened as fall-out
when working on a unified output formatter (still unfinished).

Phil Sutter (18):
  ss: Mark fall through in arg parsing switch()
  ss: Drop empty lines in UDP output
  ss: Add missing tab when printing UNIX details
  ss: Use sockstat->type in all socket types
  ss: introduce proc_ctx_print()
  ss: Drop list traversal from unix_stats_print()
  ss: Eliminate unix_use_proc()
  ss: Turn generic_proc_open() wrappers into macros
  ss: Make tmr_name local to tcp_timer_print()
  ss: Make user_ent_hash_build_init local to user_ent_hash_build()
  ss: Make some variables function-local
  ss: Make slabstat_ids local to get_slabstat()
  ss: Get rid of useless goto in handle_follow_request()
  ss: Get rid of single-fielded struct snmpstat
  ss: Make unix_state_map local to unix_show()
  ss: Make sstate_name local to sock_state_print()
  ss: Make sstate_namel local to scan_state()
  ss: unix_show: No need to initialize members of calloc'ed structs

 misc/ss.c | 524 ++++++++++++++++++++++++++------------------------------------
 1 file changed, 220 insertions(+), 304 deletions(-)

-- 
2.10.0

[iproute PATCH 01/18] ss: Mark fall through in arg parsing switch()

[Phil Sutter]

As there is a certain chance of overlooking this, better add a comment
to draw readers' attention.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/misc/ss.c b/misc/ss.c
index dd77b8153b6da..c20bfbdb01c62 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -4025,6 +4025,7 @@ int main(int argc, char *argv[])
 			exit(0);
 		case 'z':
 			show_sock_ctx++;
+			/* fall through */
 		case 'Z':
 			if (is_selinux_enabled() <= 0) {
 				fprintf(stderr, "ss: SELinux is not enabled.\n");
-- 
2.10.0

[iproute PATCH 02/18] ss: Drop empty lines in UDP output

[Phil Sutter]

When dumping UDP sockets and show_tcpinfo (-i) is active but not
show_mem (-m), print_tcpinfo() does not output anything leading to an
empty line being printed after every socket. Fix this by skipping the
call to print_tcpinfo() and the previous newline printing in that case.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/misc/ss.c b/misc/ss.c
index c20bfbdb01c62..4698683c4e84a 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2240,7 +2240,7 @@ static int inet_show_sock(struct nlmsghdr *nlh,
 		}
 	}
 
-	if (show_mem || show_tcpinfo) {
+	if (show_mem || (show_tcpinfo && protocol != IPPROTO_UDP)) {
 		printf("\n\t");
 		tcp_show_info(nlh, r, tb);
 	}
-- 
2.10.0

[iproute PATCH 03/18] ss: Add missing tab when printing UNIX details

[Phil Sutter]

When dumping UNIX sockets and show_details is active but not show_mem
(ss -xne), the socket details are printed without being prefixed by tab.
Fix this by printing the tab character when either one of '-e' or '-m'
has been specified.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 4698683c4e84a..d0b4f879c4d9f 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2907,10 +2907,10 @@ static int unix_show_sock(const struct sockaddr_nl *addr, struct nlmsghdr *nlh,
 
 	unix_stats_print(&stat, f);
 
-	if (show_mem) {
+	if (show_mem || show_details)
 		printf("\t");
+	if (show_mem)
 		print_skmeminfo(tb, UNIX_DIAG_MEMINFO);
-	}
 	if (show_details) {
 		if (tb[UNIX_DIAG_SHUTDOWN]) {
 			unsigned char mask;
-- 
2.10.0

[iproute PATCH 04/18] ss: Use sockstat->type in all socket types

[Phil Sutter]

Unix sockets used that field already to hold info about the socket type.
By replicating this approach in all other socket types, we can get rid
of protocol parameter in inet_stats_print() and have sock_state_print()
figure things out by itself.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 124 +++++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 70 insertions(+), 54 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index d0b4f879c4d9f..e6053467aaf82 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -793,8 +793,57 @@ struct tcpstat {
 	struct tcp_bbr_info *bbr_info;
 };
 
-static void sock_state_print(struct sockstat *s, const char *sock_name)
+static const char *unix_netid_name(int type)
+{
+	switch (type) {
+	case SOCK_STREAM:
+		return "u_str";
+	case SOCK_SEQPACKET:
+		return "u_seq";
+	case SOCK_DGRAM:
+	default:
+		return "u_dgr";
+	}
+}
+
+static char *proto_name(int protocol)
 {
+	switch (protocol) {
+	case 0:
+		return "raw";
+	case IPPROTO_UDP:
+		return "udp";
+	case IPPROTO_TCP:
+		return "tcp";
+	case IPPROTO_DCCP:
+		return "dccp";
+	}
+
+	return "???";
+}
+
+static void sock_state_print(struct sockstat *s)
+{
+	const char *sock_name;
+
+	switch (s->local.family) {
+	case AF_UNIX:
+		sock_name = unix_netid_name(s->type);
+		break;
+	case AF_INET:
+	case AF_INET6:
+		sock_name = proto_name(s->type);
+		break;
+	case AF_PACKET:
+		sock_name = s->type == SOCK_RAW ? "p_raw" : "p_dgr";
+		break;
+	case AF_NETLINK:
+		sock_name = "nl";
+		break;
+	default:
+		sock_name = "unknown";
+	}
+
 	if (netid_width)
 		printf("%-*s ", netid_width, sock_name);
 	if (state_width)
@@ -1670,27 +1719,11 @@ void *parse_markmask(const char *markmask)
 	return res;
 }
 
-static char *proto_name(int protocol)
-{
-	switch (protocol) {
-	case 0:
-		return "raw";
-	case IPPROTO_UDP:
-		return "udp";
-	case IPPROTO_TCP:
-		return "tcp";
-	case IPPROTO_DCCP:
-		return "dccp";
-	}
-
-	return "???";
-}
-
-static void inet_stats_print(struct sockstat *s, int protocol)
+static void inet_stats_print(struct sockstat *s)
 {
 	char *buf = NULL;
 
-	sock_state_print(s, proto_name(protocol));
+	sock_state_print(s);
 
 	inet_addr_print(&s->local, s->lport, s->iface);
 	inet_addr_print(&s->remote, s->rport, 0);
@@ -1948,8 +1981,9 @@ static int tcp_show_line(char *line, const struct filter *f, int family)
 	s.rto	    = (double)rto;
 	s.ssthresh  = s.ssthresh == -1 ? 0 : s.ssthresh;
 	s.rto	    = s.rto != 3 * hz  ? s.rto / hz : 0;
+	s.ss.type   = IPPROTO_TCP;
 
-	inet_stats_print(&s.ss, IPPROTO_TCP);
+	inet_stats_print(&s.ss);
 
 	if (show_options)
 		tcp_timer_print(&s);
@@ -2201,8 +2235,7 @@ static void parse_diag_msg(struct nlmsghdr *nlh, struct sockstat *s)
 }
 
 static int inet_show_sock(struct nlmsghdr *nlh,
-			  struct sockstat *s,
-			  int protocol)
+			  struct sockstat *s)
 {
 	struct rtattr *tb[INET_DIAG_MAX+1];
 	struct inet_diag_msg *r = NLMSG_DATA(nlh);
@@ -2211,9 +2244,9 @@ static int inet_show_sock(struct nlmsghdr *nlh,
 		     nlh->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));
 
 	if (tb[INET_DIAG_PROTOCOL])
-		protocol = *(__u8 *)RTA_DATA(tb[INET_DIAG_PROTOCOL]);
+		s->type = *(__u8 *)RTA_DATA(tb[INET_DIAG_PROTOCOL]);
 
-	inet_stats_print(s, protocol);
+	inet_stats_print(s);
 
 	if (show_options) {
 		struct tcpstat t = {};
@@ -2240,7 +2273,7 @@ static int inet_show_sock(struct nlmsghdr *nlh,
 		}
 	}
 
-	if (show_mem || (show_tcpinfo && protocol != IPPROTO_UDP)) {
+	if (show_mem || (show_tcpinfo && s->type != IPPROTO_UDP)) {
 		printf("\n\t");
 		tcp_show_info(nlh, r, tb);
 	}
@@ -2414,6 +2447,7 @@ static int show_one_inet_sock(const struct sockaddr_nl *addr,
 		return 0;
 
 	parse_diag_msg(h, &s);
+	s.type = diag_arg->protocol;
 
 	if (diag_arg->f->f && run_ssfilter(diag_arg->f->f, &s) == 0)
 		return 0;
@@ -2428,7 +2462,7 @@ static int show_one_inet_sock(const struct sockaddr_nl *addr,
 		}
 	}
 
-	err = inet_show_sock(h, &s, diag_arg->protocol);
+	err = inet_show_sock(h, &s);
 	if (err < 0)
 		return err;
 
@@ -2534,11 +2568,12 @@ static int tcp_show_netlink_file(struct filter *f)
 		}
 
 		parse_diag_msg(h, &s);
+		s.type = IPPROTO_TCP;
 
 		if (f && f->f && run_ssfilter(f->f, &s) == 0)
 			continue;
 
-		err = inet_show_sock(h, &s, IPPROTO_TCP);
+		err = inet_show_sock(h, &s);
 		if (err < 0)
 			return err;
 	}
@@ -2657,7 +2692,8 @@ static int dgram_show_line(char *line, const struct filter *f, int family)
 	if (n < 9)
 		opt[0] = 0;
 
-	inet_stats_print(&s, dg_proto == UDP_PROTO ? IPPROTO_UDP : 0);
+	s.type = dg_proto == UDP_PROTO ? IPPROTO_UDP : 0;
+	inet_stats_print(&s);
 
 	if (show_details && opt[0])
 		printf(" opt:\"%s\"", opt);
@@ -2758,25 +2794,6 @@ static void unix_list_free(struct sockstat *list)
 	}
 }
 
-static const char *unix_netid_name(int type)
-{
-	const char *netid;
-
-	switch (type) {
-	case SOCK_STREAM:
-		netid = "u_str";
-		break;
-	case SOCK_SEQPACKET:
-		netid = "u_seq";
-		break;
-	case SOCK_DGRAM:
-	default:
-		netid = "u_dgr";
-		break;
-	}
-	return netid;
-}
-
 static bool unix_type_skip(struct sockstat *s, struct filter *f)
 {
 	if (s->type == SOCK_STREAM && !(f->dbs&(1<<UNIX_ST_DB)))
@@ -2839,7 +2856,7 @@ static void unix_stats_print(struct sockstat *list, struct filter *f)
 				continue;
 		}
 
-		sock_state_print(s, unix_netid_name(s->type));
+		sock_state_print(s);
 
 		sock_addr_print(s->name ?: "*", " ",
 				int_to_str(s->lport, port_name), NULL);
@@ -3058,15 +3075,15 @@ static int packet_stats_print(struct sockstat *s, const struct filter *f)
 	const char *addr, *port;
 	char ll_name[16];
 
+	s->local.family = s->remote.family = AF_PACKET;
+
 	if (f->f) {
-		s->local.family = AF_PACKET;
-		s->remote.family = AF_PACKET;
 		s->local.data[0] = s->prot;
 		if (run_ssfilter(f->f, s) == 0)
 			return 1;
 	}
 
-	sock_state_print(s, s->type == SOCK_RAW ? "p_raw" : "p_dgr");
+	sock_state_print(s);
 
 	if (s->prot == 3)
 		addr = "*";
@@ -3316,10 +3333,9 @@ static int netlink_show_one(struct filter *f,
 	st.state = SS_CLOSE;
 	st.rq	 = rq;
 	st.wq	 = wq;
+	st.local.family = st.remote.family = AF_NETLINK;
 
 	if (f->f) {
-		st.local.family = AF_NETLINK;
-		st.remote.family = AF_NETLINK;
 		st.rport = -1;
 		st.lport = pid;
 		st.local.data[0] = prot;
@@ -3327,7 +3343,7 @@ static int netlink_show_one(struct filter *f,
 			return 1;
 	}
 
-	sock_state_print(&st, "nl");
+	sock_state_print(&st);
 
 	if (resolve_services)
 		prot_name = nl_proto_n2a(prot, prot_buf, sizeof(prot_buf));
-- 
2.10.0

Re: [iproute PATCH 04/18] ss: Use sockstat->type in all socket types

[Stephen Hemminger]

On Fri, 11 Nov 2016 14:10:00 +0100
Phil Sutter <phil@nwl.cc> wrote:

> Unix sockets used that field already to hold info about the socket type.
> By replicating this approach in all other socket types, we can get rid
> of protocol parameter in inet_stats_print() and have sock_state_print()
> figure things out by itself.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Cleaning out the patch backlog...
This patch does not apply to current master branch, rejects are:

--- misc/ss.c
+++ misc/ss.c
@@ -1719,27 +1768,11 @@ void *parse_markmask(const char *markmask)
 	return res;
 }
 
-static char *proto_name(int protocol)
-{
-	switch (protocol) {
-	case 0:
-		return "raw";
-	case IPPROTO_UDP:
-		return "udp";
-	case IPPROTO_TCP:
-		return "tcp";
-	case IPPROTO_DCCP:
-		return "dccp";
-	}
-
-	return "???";
-}
-
-static void inet_stats_print(struct sockstat *s, int protocol)
+static void inet_stats_print(struct sockstat *s)
 {
 	char *buf = NULL;
 
-	sock_state_print(s, proto_name(protocol));
+	sock_state_print(s);
 
 	inet_addr_print(&s->local, s->lport, s->iface);
 	inet_addr_print(&s->remote, s->rport, 0);


Please fix and resubmit whole series.

[iproute PATCH 05/18] ss: introduce proc_ctx_print()

[Phil Sutter]

This consolidates identical code in three places. While the function
name is not quite perfect as there is different proc_ctx printing code
in netlink_show_one() as well, I sadly didn't find a more suitable one.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 49 ++++++++++++++-----------------------------------
 1 file changed, 14 insertions(+), 35 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index e6053467aaf82..b3475cc96ae7b 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1719,14 +1719,9 @@ void *parse_markmask(const char *markmask)
 	return res;
 }
 
-static void inet_stats_print(struct sockstat *s)
+static void proc_ctx_print(struct sockstat *s)
 {
-	char *buf = NULL;
-
-	sock_state_print(s);
-
-	inet_addr_print(&s->local, s->lport, s->iface);
-	inet_addr_print(&s->remote, s->rport, 0);
+	char *buf;
 
 	if (show_proc_ctx || show_sock_ctx) {
 		if (find_entry(s->ino, &buf,
@@ -1743,6 +1738,16 @@ static void inet_stats_print(struct sockstat *s)
 	}
 }
 
+static void inet_stats_print(struct sockstat *s)
+{
+	sock_state_print(s);
+
+	inet_addr_print(&s->local, s->lport, s->iface);
+	inet_addr_print(&s->remote, s->rport, 0);
+
+	proc_ctx_print(s);
+}
+
 static int proc_parse_inet_addr(char *loc, char *rem, int family, struct
 		sockstat * s)
 {
@@ -2814,7 +2819,6 @@ static void unix_stats_print(struct sockstat *list, struct filter *f)
 {
 	struct sockstat *s;
 	char *peer;
-	char *ctx_buf = NULL;
 	bool use_proc = unix_use_proc();
 	char port_name[30] = {};
 
@@ -2863,19 +2867,7 @@ static void unix_stats_print(struct sockstat *list, struct filter *f)
 		sock_addr_print(peer, " ", int_to_str(s->rport, port_name),
 				NULL);
 
-		if (show_proc_ctx || show_sock_ctx) {
-			if (find_entry(s->ino, &ctx_buf,
-					(show_proc_ctx & show_sock_ctx) ?
-					PROC_SOCK_CTX : PROC_CTX) > 0) {
-				printf(" users:(%s)", ctx_buf);
-				free(ctx_buf);
-			}
-		} else if (show_users) {
-			if (find_entry(s->ino, &ctx_buf, USERS) > 0) {
-				printf(" users:(%s)", ctx_buf);
-				free(ctx_buf);
-			}
-		}
+		proc_ctx_print(s);
 		printf("\n");
 	}
 }
@@ -3071,7 +3063,6 @@ static int unix_show(struct filter *f)
 
 static int packet_stats_print(struct sockstat *s, const struct filter *f)
 {
-	char *buf = NULL;
 	const char *addr, *port;
 	char ll_name[16];
 
@@ -3098,19 +3089,7 @@ static int packet_stats_print(struct sockstat *s, const struct filter *f)
 	sock_addr_print(addr, ":", port, NULL);
 	sock_addr_print("", "*", "", NULL);
 
-	if (show_proc_ctx || show_sock_ctx) {
-		if (find_entry(s->ino, &buf,
-					(show_proc_ctx & show_sock_ctx) ?
-					PROC_SOCK_CTX : PROC_CTX) > 0) {
-			printf(" users:(%s)", buf);
-			free(buf);
-		}
-	} else if (show_users) {
-		if (find_entry(s->ino, &buf, USERS) > 0) {
-			printf(" users:(%s)", buf);
-			free(buf);
-		}
-	}
+	proc_ctx_print(s);
 
 	if (show_details)
 		sock_details_print(s);
-- 
2.10.0

[iproute PATCH 06/18] ss: Drop list traversal from unix_stats_print()

[Phil Sutter]

Although this complicates the dedicated procfs-based code path in
unix_show() a bit, it's the only sane way to get rid of unix_show_sock()
output diverging from other socket types in that it prints all socket
details in a new line.

As a side effect, it allows to eliminate all procfs specific code in
the same function.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 137 +++++++++++++++++++++++++++++---------------------------------
 1 file changed, 64 insertions(+), 73 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index b3475cc96ae7b..36b18ff2ce3cb 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2788,15 +2788,13 @@ int unix_state_map[] = { SS_CLOSE, SS_SYN_SENT,
 
 #define MAX_UNIX_REMEMBER (1024*1024/sizeof(struct sockstat))
 
-static void unix_list_free(struct sockstat *list)
+static void unix_list_drop_first(struct sockstat **list)
 {
-	while (list) {
-		struct sockstat *s = list;
+	struct sockstat *s = *list;
 
-		list = list->next;
-		free(s->name);
-		free(s);
-	}
+	(*list) = (*list)->next;
+	free(s->name);
+	free(s);
 }
 
 static bool unix_type_skip(struct sockstat *s, struct filter *f)
@@ -2815,61 +2813,18 @@ static bool unix_use_proc(void)
 	return getenv("PROC_NET_UNIX") || getenv("PROC_ROOT");
 }
 
-static void unix_stats_print(struct sockstat *list, struct filter *f)
+static void unix_stats_print(struct sockstat *s, struct filter *f)
 {
-	struct sockstat *s;
-	char *peer;
-	bool use_proc = unix_use_proc();
 	char port_name[30] = {};
 
-	for (s = list; s; s = s->next) {
-		if (!(f->states & (1 << s->state)))
-			continue;
-		if (unix_type_skip(s, f))
-			continue;
-
-		peer = "*";
-		if (s->peer_name)
-			peer = s->peer_name;
-
-		if (s->rport && use_proc) {
-			struct sockstat *p;
-
-			for (p = list; p; p = p->next) {
-				if (s->rport == p->lport)
-					break;
-			}
-
-			if (!p) {
-				peer = "?";
-			} else {
-				peer = p->name ? : "*";
-			}
-		}
-
-		if (use_proc && f->f) {
-			struct sockstat st = {
-				.local.family = AF_UNIX,
-				.remote.family = AF_UNIX,
-			};
-
-			memcpy(st.local.data, &s->name, sizeof(s->name));
-			if (strcmp(peer, "*"))
-				memcpy(st.remote.data, &peer, sizeof(peer));
-			if (run_ssfilter(f->f, &st) == 0)
-				continue;
-		}
-
-		sock_state_print(s);
+	sock_state_print(s);
 
-		sock_addr_print(s->name ?: "*", " ",
-				int_to_str(s->lport, port_name), NULL);
-		sock_addr_print(peer, " ", int_to_str(s->rport, port_name),
-				NULL);
+	sock_addr_print(s->name ?: "*", " ",
+			int_to_str(s->lport, port_name), NULL);
+	sock_addr_print(s->peer_name ?: "*", " ",
+			int_to_str(s->rport, port_name), NULL);
 
-		proc_ctx_print(s);
-		printf("\n");
-	}
+	proc_ctx_print(s);
 }
 
 static int unix_show_sock(const struct sockaddr_nl *addr, struct nlmsghdr *nlh,
@@ -2916,8 +2871,6 @@ static int unix_show_sock(const struct sockaddr_nl *addr, struct nlmsghdr *nlh,
 
 	unix_stats_print(&stat, f);
 
-	if (show_mem || show_details)
-		printf("\t");
 	if (show_mem)
 		print_skmeminfo(tb, UNIX_DIAG_MEMINFO);
 	if (show_details) {
@@ -2928,8 +2881,7 @@ static int unix_show_sock(const struct sockaddr_nl *addr, struct nlmsghdr *nlh,
 			printf(" %c-%c", mask & 1 ? '-' : '<', mask & 2 ? '-' : '>');
 		}
 	}
-	if (show_mem || show_details)
-		printf("\n");
+	printf("\n");
 
 	return 0;
 }
@@ -3020,6 +2972,11 @@ static int unix_show(struct filter *f)
 			if (u->type == SOCK_DGRAM && u->state == SS_CLOSE && u->rport)
 				u->state = SS_ESTABLISHED;
 		}
+		if (unix_type_skip(u, f) ||
+		    !(f->states & (1 << u->state))) {
+			free(u);
+			continue;
+		}
 
 		if (!newformat) {
 			u->rport = 0;
@@ -3027,6 +2984,42 @@ static int unix_show(struct filter *f)
 			u->wq = 0;
 		}
 
+		if (name[0]) {
+			u->name = strdup(name);
+			if (!u->name)
+				break;
+		}
+
+		if (u->rport) {
+			struct sockstat *p;
+
+			for (p = list; p; p = p->next) {
+				if (u->rport == p->lport)
+					break;
+			}
+			if (!p)
+				u->peer_name = "?";
+			else
+				u->peer_name = p->name ? : "*";
+		}
+
+		if (f->f) {
+			struct sockstat st = {
+				.local.family = AF_UNIX,
+				.remote.family = AF_UNIX,
+			};
+
+			memcpy(st.local.data, &u->name, sizeof(u->name));
+			if (strcmp(u->peer_name, "*"))
+				memcpy(st.remote.data, &u->peer_name,
+				       sizeof(u->peer_name));
+			if (run_ssfilter(f->f, &st) == 0) {
+				free(u->name);
+				free(u);
+				continue;
+			}
+		}
+
 		insp = &list;
 		while (*insp) {
 			if (u->type < (*insp)->type ||
@@ -3038,24 +3031,22 @@ static int unix_show(struct filter *f)
 		u->next = *insp;
 		*insp = u;
 
-		if (name[0]) {
-			if ((u->name = malloc(strlen(name)+1)) == NULL)
-				break;
-			strcpy(u->name, name);
-		}
 		if (++cnt > MAX_UNIX_REMEMBER) {
-			unix_stats_print(list, f);
-			unix_list_free(list);
-			list = NULL;
+			while (list) {
+				unix_stats_print(list, f);
+				printf("\n");
+
+				unix_list_drop_first(&list);
+			}
 			cnt = 0;
 		}
 	}
 	fclose(fp);
-	if (list) {
+	while (list) {
 		unix_stats_print(list, f);
-		unix_list_free(list);
-		list = NULL;
-		cnt = 0;
+		printf("\n");
+
+		unix_list_drop_first(&list);
 	}
 
 	return 0;
-- 
2.10.0

[iproute PATCH 07/18] ss: Eliminate unix_use_proc()

[Phil Sutter]

This function is used only at a single place anymore, so replace the
call to it by it's content, which makes that specific part of
unix_show() consistent with e.g. tcp_show().

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 36b18ff2ce3cb..8e021731cf71c 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2808,11 +2808,6 @@ static bool unix_type_skip(struct sockstat *s, struct filter *f)
 	return false;
 }
 
-static bool unix_use_proc(void)
-{
-	return getenv("PROC_NET_UNIX") || getenv("PROC_ROOT");
-}
-
 static void unix_stats_print(struct sockstat *s, struct filter *f)
 {
 	char port_name[30] = {};
@@ -2934,7 +2929,8 @@ static int unix_show(struct filter *f)
 	if (!filter_af_get(f, AF_UNIX))
 		return 0;
 
-	if (!unix_use_proc() && unix_show_netlink(f) == 0)
+	if (!getenv("PROC_NET_UNIX") && !getenv("PROC_ROOT")
+	    && unix_show_netlink(f) == 0)
 		return 0;
 
 	if ((fp = net_unix_open()) == NULL)
-- 
2.10.0

[iproute PATCH 08/18] ss: Turn generic_proc_open() wrappers into macros

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 89 ++++++++++++++-------------------------------------------------
 1 file changed, 19 insertions(+), 70 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 8e021731cf71c..e9fecd39a8493 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -306,76 +306,25 @@ static FILE *generic_proc_open(const char *env, const char *name)
 
 	return fopen(p, "r");
 }
-
-static FILE *net_tcp_open(void)
-{
-	return generic_proc_open("PROC_NET_TCP", "net/tcp");
-}
-
-static FILE *net_tcp6_open(void)
-{
-	return generic_proc_open("PROC_NET_TCP6", "net/tcp6");
-}
-
-static FILE *net_udp_open(void)
-{
-	return generic_proc_open("PROC_NET_UDP", "net/udp");
-}
-
-static FILE *net_udp6_open(void)
-{
-	return generic_proc_open("PROC_NET_UDP6", "net/udp6");
-}
-
-static FILE *net_raw_open(void)
-{
-	return generic_proc_open("PROC_NET_RAW", "net/raw");
-}
-
-static FILE *net_raw6_open(void)
-{
-	return generic_proc_open("PROC_NET_RAW6", "net/raw6");
-}
-
-static FILE *net_unix_open(void)
-{
-	return generic_proc_open("PROC_NET_UNIX", "net/unix");
-}
-
-static FILE *net_packet_open(void)
-{
-	return generic_proc_open("PROC_NET_PACKET", "net/packet");
-}
-
-static FILE *net_netlink_open(void)
-{
-	return generic_proc_open("PROC_NET_NETLINK", "net/netlink");
-}
-
-static FILE *slabinfo_open(void)
-{
-	return generic_proc_open("PROC_SLABINFO", "slabinfo");
-}
-
-static FILE *net_sockstat_open(void)
-{
-	return generic_proc_open("PROC_NET_SOCKSTAT", "net/sockstat");
-}
-
-static FILE *net_sockstat6_open(void)
-{
-	return generic_proc_open("PROC_NET_SOCKSTAT6", "net/sockstat6");
-}
-
-static FILE *net_snmp_open(void)
-{
-	return generic_proc_open("PROC_NET_SNMP", "net/snmp");
-}
-
-static FILE *ephemeral_ports_open(void)
-{
-	return generic_proc_open("PROC_IP_LOCAL_PORT_RANGE", "sys/net/ipv4/ip_local_port_range");
-}
+#define net_tcp_open()		generic_proc_open("PROC_NET_TCP", "net/tcp")
+#define net_tcp6_open()		generic_proc_open("PROC_NET_TCP6", "net/tcp6")
+#define net_udp_open()		generic_proc_open("PROC_NET_UDP", "net/udp")
+#define net_udp6_open()		generic_proc_open("PROC_NET_UDP6", "net/udp6")
+#define net_raw_open()		generic_proc_open("PROC_NET_RAW", "net/raw")
+#define net_raw6_open()		generic_proc_open("PROC_NET_RAW6", "net/raw6")
+#define net_unix_open()		generic_proc_open("PROC_NET_UNIX", "net/unix")
+#define net_packet_open()	generic_proc_open("PROC_NET_PACKET", \
+							"net/packet")
+#define net_netlink_open()	generic_proc_open("PROC_NET_NETLINK", \
+							"net/netlink")
+#define slabinfo_open()		generic_proc_open("PROC_SLABINFO", "slabinfo")
+#define net_sockstat_open()	generic_proc_open("PROC_NET_SOCKSTAT", \
+							"net/sockstat")
+#define net_sockstat6_open()	generic_proc_open("PROC_NET_SOCKSTAT6", \
+							"net/sockstat6")
+#define net_snmp_open()		generic_proc_open("PROC_NET_SNMP", "net/snmp")
+#define ephemeral_ports_open()	generic_proc_open("PROC_IP_LOCAL_PORT_RANGE", \
+					"sys/net/ipv4/ip_local_port_range")
 
 struct user_ent {
 	struct user_ent	*next;
-- 
2.10.0

[iproute PATCH 09/18] ss: Make tmr_name local to tcp_timer_print()

[Phil Sutter]

It's used only there, so no need to have it globally defined.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index e9fecd39a8493..477910a842726 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -830,15 +830,6 @@ static void sock_addr_print(const char *addr, char *delim, const char *port,
 	sock_addr_print_width(addr_width, addr, delim, serv_width, port, ifname);
 }
 
-static const char *tmr_name[] = {
-	"off",
-	"on",
-	"keepalive",
-	"timewait",
-	"persist",
-	"unknown"
-};
-
 static const char *print_ms_timer(int timeout)
 {
 	static char buf[64];
@@ -1879,6 +1870,15 @@ static void tcp_stats_print(struct tcpstat *s)
 
 static void tcp_timer_print(struct tcpstat *s)
 {
+	static const char * const tmr_name[] = {
+		"off",
+		"on",
+		"keepalive",
+		"timewait",
+		"persist",
+		"unknown"
+	};
+
 	if (s->timer) {
 		if (s->timer > 4)
 			s->timer = 5;
-- 
2.10.0

[iproute PATCH 10/18] ss: Make user_ent_hash_build_init local to user_ent_hash_build()

[Phil Sutter]

By having it statically defined, there is no need for it to be global.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 477910a842726..3e5c93bb7c6f9 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -99,8 +99,6 @@ int show_bpf;
 int show_proc_ctx;
 int show_sock_ctx;
 int show_header = 1;
-/* If show_users & show_proc_ctx only do user_ent_hash_build() once */
-int user_ent_hash_build_init;
 int follow_events;
 
 int netid_width;
@@ -400,6 +398,7 @@ static void user_ent_hash_build(void)
 	char *pid_context;
 	char *sock_context;
 	const char *no_ctx = "unavailable";
+	static int user_ent_hash_build_init;
 
 	/* If show_users & show_proc_ctx set only do this once */
 	if (user_ent_hash_build_init != 0)
-- 
2.10.0

[iproute PATCH 11/18] ss: Make some variables function-local

[Phil Sutter]

addrp_width and screen_width are used in main() only, so no need to have
them globally available.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 3e5c93bb7c6f9..d546a00eb2c24 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -103,10 +103,8 @@ int follow_events;
 
 int netid_width;
 int state_width;
-int addrp_width;
 int addr_width;
 int serv_width;
-int screen_width;
 
 static const char *TCP_PROTO = "tcp";
 static const char *UDP_PROTO = "udp";
@@ -3784,6 +3782,7 @@ int main(int argc, char *argv[])
 	FILE *filter_fp = NULL;
 	int ch;
 	int state_filter = 0;
+	int addrp_width, screen_width = 80;
 
 	while ((ch = getopt_long(argc, argv, "dhaletuwxnro460spbEf:miA:D:F:vVzZN:KH",
 				 long_opts, NULL)) != EOF) {
@@ -4067,7 +4066,6 @@ int main(int argc, char *argv[])
 	if (current_filter.states&(current_filter.states-1))
 		state_width = 10;
 
-	screen_width = 80;
 	if (isatty(STDOUT_FILENO)) {
 		struct winsize w;
 
-- 
2.10.0

[iproute PATCH 12/18] ss: Make slabstat_ids local to get_slabstat()

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index d546a00eb2c24..c3a5148e05013 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -580,21 +580,19 @@ struct slabstat {
 
 static struct slabstat slabstat;
 
-static const char *slabstat_ids[] = {
-
-	"sock",
-	"tcp_bind_bucket",
-	"tcp_tw_bucket",
-	"tcp_open_request",
-	"skbuff_head_cache",
-};
-
 static int get_slabstat(struct slabstat *s)
 {
 	char buf[256];
 	FILE *fp;
 	int cnt;
 	static int slabstat_valid;
+	static const char * const slabstat_ids[] = {
+		"sock",
+		"tcp_bind_bucket",
+		"tcp_tw_bucket",
+		"tcp_open_request",
+		"skbuff_head_cache",
+	};
 
 	if (slabstat_valid)
 		return 0;
-- 
2.10.0

[iproute PATCH 13/18] ss: Get rid of useless goto in handle_follow_request()

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index c3a5148e05013..b06b9e6fa9884 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -3443,7 +3443,7 @@ static int generic_show_sock(const struct sockaddr_nl *addr,
 
 static int handle_follow_request(struct filter *f)
 {
-	int ret = -1;
+	int ret = 0;
 	int groups = 0;
 	struct rtnl_handle rth;
 
@@ -3466,10 +3466,8 @@ static int handle_follow_request(struct filter *f)
 	rth.local.nl_pid = 0;
 
 	if (rtnl_dump_filter(&rth, generic_show_sock, f))
-		goto Exit;
+		ret = -1;
 
-	ret = 0;
-Exit:
 	rtnl_close(&rth);
 	return ret;
 }
-- 
2.10.0

[iproute PATCH 14/18] ss: Get rid of single-fielded struct snmpstat

[Phil Sutter]

A struct with only a single field does not make much sense. Besides
that, it was used by print_summary() only.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index b06b9e6fa9884..85fc6096a986f 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -3472,10 +3472,6 @@ static int handle_follow_request(struct filter *f)
 	return ret;
 }
 
-struct snmpstat {
-	int tcp_estab;
-};
-
 static int get_snmp_int(char *proto, char *key, int *result)
 {
 	char buf[1024];
@@ -3596,11 +3592,11 @@ static int get_sockstat(struct ssummary *s)
 static int print_summary(void)
 {
 	struct ssummary s;
-	struct snmpstat sn;
+	int tcp_estab;
 
 	if (get_sockstat(&s) < 0)
 		perror("ss: get_sockstat");
-	if (get_snmp_int("Tcp:", "CurrEstab", &sn.tcp_estab) < 0)
+	if (get_snmp_int("Tcp:", "CurrEstab", &tcp_estab) < 0)
 		perror("ss: get_snmpstat");
 
 	get_slabstat(&slabstat);
@@ -3609,7 +3605,7 @@ static int print_summary(void)
 
 	printf("TCP:   %d (estab %d, closed %d, orphaned %d, synrecv %d, timewait %d/%d), ports %d\n",
 	       s.tcp_total + slabstat.tcp_syns + s.tcp_tws,
-	       sn.tcp_estab,
+	       tcp_estab,
 	       s.tcp_total - (s.tcp4_hashed+s.tcp6_hashed-s.tcp_tws),
 	       s.tcp_orphans,
 	       slabstat.tcp_syns,
-- 
2.10.0

[iproute PATCH 15/18] ss: Make unix_state_map local to unix_show()

[Phil Sutter]

Also make it const, since there won't be any write access happening.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 85fc6096a986f..cf4187310816e 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2727,9 +2727,6 @@ outerr:
 	} while (0);
 }
 
-int unix_state_map[] = { SS_CLOSE, SS_SYN_SENT,
-			 SS_ESTABLISHED, SS_CLOSING };
-
 #define MAX_UNIX_REMEMBER (1024*1024/sizeof(struct sockstat))
 
 static void unix_list_drop_first(struct sockstat **list)
@@ -2869,6 +2866,8 @@ static int unix_show(struct filter *f)
 	int  newformat = 0;
 	int  cnt;
 	struct sockstat *list = NULL;
+	const int unix_state_map[] = { SS_CLOSE, SS_SYN_SENT,
+				       SS_ESTABLISHED, SS_CLOSING };
 
 	if (!filter_af_get(f, AF_UNIX))
 		return 0;
-- 
2.10.0

[iproute PATCH 16/18] ss: Make sstate_name local to sock_state_print()

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index cf4187310816e..14492da256c61 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -634,21 +634,6 @@ static unsigned long long cookie_sk_get(const uint32_t *cookie)
 	return (((unsigned long long)cookie[1] << 31) << 1) | cookie[0];
 }
 
-static const char *sstate_name[] = {
-	"UNKNOWN",
-	[SS_ESTABLISHED] = "ESTAB",
-	[SS_SYN_SENT] = "SYN-SENT",
-	[SS_SYN_RECV] = "SYN-RECV",
-	[SS_FIN_WAIT1] = "FIN-WAIT-1",
-	[SS_FIN_WAIT2] = "FIN-WAIT-2",
-	[SS_TIME_WAIT] = "TIME-WAIT",
-	[SS_CLOSE] = "UNCONN",
-	[SS_CLOSE_WAIT] = "CLOSE-WAIT",
-	[SS_LAST_ACK] = "LAST-ACK",
-	[SS_LISTEN] =	"LISTEN",
-	[SS_CLOSING] = "CLOSING",
-};
-
 static const char *sstate_namel[] = {
 	"UNKNOWN",
 	[SS_ESTABLISHED] = "established",
@@ -769,6 +754,20 @@ static char *proto_name(int protocol)
 static void sock_state_print(struct sockstat *s)
 {
 	const char *sock_name;
+	static const char * const sstate_name[] = {
+		"UNKNOWN",
+		[SS_ESTABLISHED] = "ESTAB",
+		[SS_SYN_SENT] = "SYN-SENT",
+		[SS_SYN_RECV] = "SYN-RECV",
+		[SS_FIN_WAIT1] = "FIN-WAIT-1",
+		[SS_FIN_WAIT2] = "FIN-WAIT-2",
+		[SS_TIME_WAIT] = "TIME-WAIT",
+		[SS_CLOSE] = "UNCONN",
+		[SS_CLOSE_WAIT] = "CLOSE-WAIT",
+		[SS_LAST_ACK] = "LAST-ACK",
+		[SS_LISTEN] =	"LISTEN",
+		[SS_CLOSING] = "CLOSING",
+	};
 
 	switch (s->local.family) {
 	case AF_UNIX:
-- 
2.10.0

[iproute PATCH 17/18] ss: Make sstate_namel local to scan_state()

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 14492da256c61..6e669f7b0593c 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -634,21 +634,6 @@ static unsigned long long cookie_sk_get(const uint32_t *cookie)
 	return (((unsigned long long)cookie[1] << 31) << 1) | cookie[0];
 }
 
-static const char *sstate_namel[] = {
-	"UNKNOWN",
-	[SS_ESTABLISHED] = "established",
-	[SS_SYN_SENT] = "syn-sent",
-	[SS_SYN_RECV] = "syn-recv",
-	[SS_FIN_WAIT1] = "fin-wait-1",
-	[SS_FIN_WAIT2] = "fin-wait-2",
-	[SS_TIME_WAIT] = "time-wait",
-	[SS_CLOSE] = "unconnected",
-	[SS_CLOSE_WAIT] = "close-wait",
-	[SS_LAST_ACK] = "last-ack",
-	[SS_LISTEN] =	"listening",
-	[SS_CLOSING] = "closing",
-};
-
 struct sockstat {
 	struct sockstat	   *next;
 	unsigned int	    type;
@@ -3698,6 +3683,20 @@ static void usage(void)
 
 static int scan_state(const char *state)
 {
+	static const char * const sstate_namel[] = {
+		"UNKNOWN",
+		[SS_ESTABLISHED] = "established",
+		[SS_SYN_SENT] = "syn-sent",
+		[SS_SYN_RECV] = "syn-recv",
+		[SS_FIN_WAIT1] = "fin-wait-1",
+		[SS_FIN_WAIT2] = "fin-wait-2",
+		[SS_TIME_WAIT] = "time-wait",
+		[SS_CLOSE] = "unconnected",
+		[SS_CLOSE_WAIT] = "close-wait",
+		[SS_LAST_ACK] = "last-ack",
+		[SS_LISTEN] =	"listening",
+		[SS_CLOSING] = "closing",
+	};
 	int i;
 
 	if (strcasecmp(state, "close") == 0 ||
-- 
2.10.0

[iproute PATCH 18/18] ss: unix_show: No need to initialize members of calloc'ed structs

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index 6e669f7b0593c..1e3ccf28c4e84 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2877,8 +2877,6 @@ static int unix_show(struct filter *f)
 
 		if (!(u = calloc(1, sizeof(*u))))
 			break;
-		u->name = NULL;
-		u->peer_name = NULL;
 
 		if (sscanf(buf, "%x: %x %x %x %x %x %d %s",
 			   &u->rport, &u->rq, &u->wq, &flags, &u->type,
-- 
2.10.0