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

[iproute PATCH v2 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).

Changes since v1:
- Rebased onto current upstream, resolved conflicts in patch 4 generated
  by previously added SCTP socket support.

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 | 532 ++++++++++++++++++++++++++------------------------------------
 1 file changed, 224 insertions(+), 308 deletions(-)

-- 
2.10.0

[iproute PATCH v2 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 07dcd8c209c04..469721fd9aee3 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -4223,6 +4223,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 v2 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 469721fd9aee3..3871a6f61f8ea 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2412,7 +2412,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");
 		if (protocol == IPPROTO_SCTP)
 			sctp_show_info(nlh, r, tb);
-- 
2.10.0

[iproute PATCH v2 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 3871a6f61f8ea..f1053b1db4132 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -3096,10 +3096,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 v2 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 | 132 +++++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 74 insertions(+), 58 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index f1053b1db4132..a953d4b022aed 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -837,8 +837,59 @@ static bool is_sctp_assoc(struct sockstat *s, const char *sock_name)
 	return true;
 }
 
-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 const char *proto_name(int protocol)
+{
+	switch (protocol) {
+	case 0:
+		return "raw";
+	case IPPROTO_UDP:
+		return "udp";
+	case IPPROTO_TCP:
+		return "tcp";
+	case IPPROTO_SCTP:
+		return "sctp";
+	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,
 		       is_sctp_assoc(s, sock_name) ? "" : sock_name);
@@ -1722,29 +1773,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_SCTP:
-		return "sctp";
-	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);
@@ -2059,8 +2092,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);
@@ -2370,8 +2404,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);
@@ -2380,9 +2413,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 = {};
@@ -2390,7 +2423,7 @@ static int inet_show_sock(struct nlmsghdr *nlh,
 		t.timer = r->idiag_timer;
 		t.timeout = r->idiag_expires;
 		t.retrans = r->idiag_retrans;
-		if (protocol == IPPROTO_SCTP)
+		if (s->type == IPPROTO_SCTP)
 			sctp_timer_print(&t);
 		else
 			tcp_timer_print(&t);
@@ -2412,9 +2445,9 @@ 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");
-		if (protocol == IPPROTO_SCTP)
+		if (s->type == IPPROTO_SCTP)
 			sctp_show_info(nlh, r, tb);
 		else
 			tcp_show_info(nlh, r, tb);
@@ -2590,6 +2623,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;
@@ -2604,7 +2638,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;
 
@@ -2710,11 +2744,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;
 	}
@@ -2844,7 +2879,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);
@@ -2945,25 +2981,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)))
@@ -3026,7 +3043,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);
@@ -3247,15 +3264,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 = "*";
@@ -3505,10 +3522,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;
@@ -3516,7 +3532,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

[iproute PATCH v2 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 a953d4b022aed..fcbaecbe25a2f 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1773,14 +1773,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,
@@ -1797,6 +1792,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)
 {
@@ -3001,7 +3006,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] = {};
 
@@ -3050,19 +3054,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");
 	}
 }
@@ -3260,7 +3252,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];
 
@@ -3287,19 +3278,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 v2 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 fcbaecbe25a2f..0de336200142f 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2975,15 +2975,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)
@@ -3002,61 +3000,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,
@@ -3105,8 +3060,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) {
@@ -3117,8 +3070,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;
 }
@@ -3209,6 +3161,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;
@@ -3216,6 +3173,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 ||
@@ -3227,24 +3220,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 v2 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 0de336200142f..ad38eb97b0055 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2995,11 +2995,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] = {};
@@ -3123,7 +3118,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 v2 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 ad38eb97b0055..71040a82ca6b1 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -327,76 +327,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 v2 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 71040a82ca6b1..97fcfd4a85548 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -882,15 +882,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];
@@ -1983,6 +1974,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 v2 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 97fcfd4a85548..44386c82c7578 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -100,8 +100,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 sctp_ino;
 
@@ -421,6 +419,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 v2 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 44386c82c7578..3662f5f4861c7 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -105,10 +105,8 @@ int sctp_ino;
 
 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 *SCTP_PROTO = "sctp";
@@ -3975,6 +3973,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:KHS",
@@ -4264,7 +4263,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 v2 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 3662f5f4861c7..c498478421190 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -601,21 +601,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 v2 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 c498478421190..ec71c21ce6a4a 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -3632,7 +3632,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;
 
@@ -3655,10 +3655,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 v2 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 ec71c21ce6a4a..c7818eadf9e75 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -3661,10 +3661,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];
@@ -3785,11 +3781,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);
@@ -3798,7 +3794,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 v2 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 c7818eadf9e75..e82c416b5fa72 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -2914,9 +2914,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)
@@ -3058,6 +3055,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 v2 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 e82c416b5fa72..8439f473d7f7b 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -655,21 +655,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 *sctp_sstate_name[] = {
 	[SCTP_STATE_CLOSED] = "CLOSED",
 	[SCTP_STATE_COOKIE_WAIT] = "COOKIE_WAIT",
@@ -815,6 +800,20 @@ static const 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 v2 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 8439f473d7f7b..c72aba7e65ad3 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -666,21 +666,6 @@ static const char *sctp_sstate_name[] = {
 	[SCTP_STATE_SHUTDOWN_ACK_SENT] = "ACK_SENT",
 };
 
-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;
@@ -3888,6 +3873,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 v2 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 c72aba7e65ad3..f23aa6be33174 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -3066,8 +3066,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

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

[Stephen Hemminger]

On Fri,  2 Dec 2016 11:39:44 +0100
Phil Sutter <phil@nwl.cc> wrote:

> This is a series of misc changes to ss code which happened as fall-out
> when working on a unified output formatter (still unfinished).
> 
> Changes since v1:
> - Rebased onto current upstream, resolved conflicts in patch 4 generated
>   by previously added SCTP socket support.
> 
> 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 | 532 ++++++++++++++++++++++++++------------------------------------
>  1 file changed, 224 insertions(+), 308 deletions(-)
> 

Applied, thanks

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

[David Laight]

From: Phil Sutter
> Sent: 02 December 2016 10:40
> 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 ad38eb97b0055..71040a82ca6b1 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -327,76 +327,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");
> -}
...
> +#define net_tcp_open()		generic_proc_open("PROC_NET_TCP", "net/tcp")
...

Does that make any difference at all?
The compiler should inline the static functions.

	David

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

[Phil Sutter]

On Mon, Dec 05, 2016 at 05:02:20PM +0000, David Laight wrote:
> From: Phil Sutter
> > Sent: 02 December 2016 10:40
> > 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 ad38eb97b0055..71040a82ca6b1 100644
> > --- a/misc/ss.c
> > +++ b/misc/ss.c
> > @@ -327,76 +327,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");
> > -}
> ...
> > +#define net_tcp_open()		generic_proc_open("PROC_NET_TCP", "net/tcp")
> ...
> 
> Does that make any difference at all?
> The compiler should inline the static functions.

Probably, yes. My motivation behind it was merely the reduced LoC (and
maybe the improved clarity how these are really just wrappers, but
that's personal preference I guess).

Cheers, Phil